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

[archivist] Allow defining block time in settings for archivist #32

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

jjmerchante
Copy link
Collaborator

This PR allows defining the block time for reading items from the stream in archivist jobs when no more items are available for insertion into the database.

This enables faster item insertion into storage but may cause the job to finish earlier.

Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

The PR works ok but the parameter to set the time is missing in the configuration file.

src/grimoirelab/core/runner/commands/run.py Show resolved Hide resolved
@@ -34,7 +34,7 @@


MAX_EVENTS_PER_JOB = 5000
BLOCK_TIMEOUT = 60000 # seconds
BLOCK_TIMEOUT = 60000 # milliseconds
Copy link
Member

Choose a reason for hiding this comment

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

This should be part of the settings and not part of this module.

@jjmerchante jjmerchante force-pushed the archivist-block branch 2 times, most recently from 5137003 to 8ef1da7 Compare February 12, 2025 15:54
This commit allows defining the block time for reading items
from the stream in archivist jobs when no more items are available
for insertion into the database.

This enables faster item insertion into storage but may cause the
job to finish earlier.

Signed-off-by: Jose Javier Merchante <[email protected]>
Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

LGTM

@sduenas sduenas merged commit 173152e into chaoss:main Feb 12, 2025
5 checks passed
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.

2 participants