Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make indexer mode not spawn the server specific behaviour #4072

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

wendrul
Copy link
Contributor

@wendrul wendrul commented Jul 12, 2024

see: https://github.com/windmill-labs/windmill-ee-private/pull/40

Done in this PR:

  • Add memory logging and logs during the loop to check progress.
  • functional workspace filtering
  • skip missed ticks on index refresh jobs scheduler
  • remove embeddings and other unnecessary functionality from the indexer mode

Not done:

  • Add endpoint to track number of jobs indexed and other observability endpoints
  • Add lookback period and/or row limit to create a moving window of indexed jobs
  • Progressive commit strategy to start searching while the indexer is running (Right now you must wait on the first run for the indexer to index the whole table before you can search jobs)

🚀 This description was created by Ellipsis for commit d13a0e1

Summary:

Adjusted indexer mode behavior in backend/src/main.rs to prevent spawning server-specific actions and made several improvements to the indexer functionality.

Key points:

  • Add memory logging and logs during the loop to check progress.
  • Functional workspace filtering.
  • Skip missed ticks on index refresh jobs scheduler.
  • Remove embeddings and other unnecessary functionality from the indexer mode.
  • Adjust indexer mode behavior in backend/src/main.rs to prevent spawning server-specific actions.
  • Remove indexer mode from the server_mode condition.
  • Add a separate indexer_mode condition.
  • Update server bind address logic to include indexer_mode.
  • Ensure server-specific behavior is not spawned in indexer mode.

Generated with ❤️ by ellipsis.dev

@wendrul wendrul requested a review from rubenfiszel as a code owner July 12, 2024 18:20
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to b957a59 in 41 seconds

More details
  • Looked at 28 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/src/main.rs:219
  • Draft comment:
    The condition for server_mode now excludes Mode::Indexer, which might affect configurations or initializations that depend on server_mode. Ensure that this change aligns with the intended operational differences between the server and indexer modes.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_d6GrleB6KSg31xIr


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

cloudflare-workers-and-pages bot commented Jul 12, 2024

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: d13a0e1
Status: ✅  Deploy successful!
Preview URL: https://e780fe61.windmill.pages.dev
Branch Preview URL: https://win-243-indexer-fixes.windmill.pages.dev

View logs

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on d13a0e1 in 28 seconds

More details
  • Looked at 7 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/ee-repo-ref.txt:1
  • Draft comment:
    Ensure the updated commit hash 6734a8ed5a1b87ecf63f7cd5e8529b721c69f14d in ee-repo-ref.txt corresponds to a valid and intended update in the external repository. This is crucial for maintaining consistency and stability in the backend dependencies.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The change in the ee-repo-ref.txt file is updating the reference to a new commit hash. This is a common practice to ensure that the backend is aligned with a specific state of an external repository or a submodule. It's important to verify that this new commit hash corresponds to a valid and intended update in the external repository, which could include bug fixes, feature updates, or other necessary changes. However, without access to the external repository, it's impossible to directly verify the validity and implications of this update.

Workflow ID: wflow_uh70qHyUNT7NNDu0


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel merged commit 88ae5e8 into main Jul 12, 2024
3 checks passed
@rubenfiszel rubenfiszel deleted the win-243-indexer-fixes branch July 12, 2024 18:36
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants