Skip to content

Job registry outbox design#227

Open
l1ttps wants to merge 7 commits intomainfrom
job-registry-outbox-design
Open

Job registry outbox design#227
l1ttps wants to merge 7 commits intomainfrom
job-registry-outbox-design

Conversation

@l1ttps
Copy link
Copy Markdown
Member

@l1ttps l1ttps commented Jan 7, 2026

No description provided.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @l1ttps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the job registry system by introducing an outbox pattern for reliable event publishing, improving the accuracy and detail of job status and duration displays in the console, and implementing a distributed locking mechanism with Redis for better concurrency control. Additionally, it refines workflow triggering to be workspace-specific, ensuring that automated processes are correctly scoped.

Highlights

  • Job Outbox Implementation: A new JobOutbox entity and associated logic have been introduced to implement the outbox pattern, ensuring reliable event publishing for job creation. This involves atomic saving of jobs and their corresponding outbox records within a database transaction.
  • Enhanced Job Status Display and Logic: The console's job registry now includes a 'pending' status for job tools in workflow views, improving visibility into job progression. The duration calculation has also been refined to use 'pickJobAt' for more accurate timing.
  • Distributed Locking Service: A new RedisLockService has been added, providing a robust distributed locking mechanism using Redis. This service offers methods for acquiring, checking, and releasing locks, crucial for managing concurrent operations.
  • Workspace-Aware Workflow Triggering: Workflow event listeners have been updated to consider the 'workspaceId', allowing for more precise and isolated workflow triggering based on the workspace context.
  • Processor Dependency Cleanup: Unnecessary dependencies have been removed from 'scan-schedule.processor.ts', streamlining the job processors.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an outbox pattern for job creation to improve reliability, using database transactions to ensure jobs and their outbox entries are created atomically. It also adds a distributed lock service using Redis. The frontend is updated to provide a more accurate representation of job and workflow statuses. My review includes a critical fix for the distributed lock implementation to prevent race conditions and a suggestion to improve type safety in the new outbox entity. Overall, these are great improvements for the robustness and correctness of the job registry system.

I am having trouble creating individual review comments. Click here to see my feedback.

core-api/src/services/redis/distributed-lock.service.ts (22-31)

critical

The lockWithTimeOut method is implemented using redis.client.set(...) without the NX (set if not exists) option. This means it will always overwrite any existing key, failing to function as an atomic lock acquisition. It will almost always return true, giving a false sense of having acquired a lock, which can lead to severe race conditions and data corruption. The method's documentation is also misleading. To fix this, the NX option must be added to the set command, just like in the private acquireLock method.

  public async lockWithTimeOut(key: string, ttl: number): Promise<boolean> {
    const lockValue = Date.now().toString();
    const result = await this.redisService.client.set(
      `${this.prefix}:${key}`,
      lockValue,
      'PX',
      ttl,
      'NX',
    );
    return result === 'OK';
  }

core-api/src/modules/jobs-registry/entities/job-outbox.entity.ts (16)

medium

Using any for the payload property sacrifices type safety. It's better to use a more specific type. Based on its usage, this payload seems to have a consistent structure. Consider defining a specific interface or using a DTO for it to ensure consistency and prevent potential runtime errors. As a minimal improvement, Record<string, unknown> is safer than any.

  payload: Record<string, unknown>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant