-
Notifications
You must be signed in to change notification settings - Fork 23
Enhance dque fs sync #397
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
Enhance dque fs sync #397
Conversation
|
@hyperspace-insights[bot] Command |
1 similar comment
|
@hyperspace-insights[bot] Command |
|
@hyperspace-insights[bot] You have mentioned internal references in the public. Please check. |
|
@nickytd You have mentioned internal references in the public. Please check. |
1 similar comment
|
@nickytd You have mentioned internal references in the public. Please check. |
|
@hyperspace-insights[bot] Command |
1 similar comment
|
@hyperspace-insights[bot] Command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
This pull request introduces support for seed scenario testing alongside the existing shoot scenario, refactors the performance test scripts, fixes a typo in variable naming (isStooped → isStopped), and adds queue metrics monitoring. While the changes are generally well-structured, there are several issues that need to be addressed:
Key Issues Found:
- Shell scripting best practices: Missing quotes around variable expansions in function calls could lead to word splitting issues
- Makefile variable quoting: The SCENARIO variable has unnecessary quotes that could cause comparison issues
- Missing newline at EOF: The
down.shfile lacks a trailing newline, which violates POSIX standards - Performance concern: The timer implementation in
dque.gouses a select with default case in a tight loop, which could cause unnecessary CPU overhead - Error handling: The
TurboSync()error is only logged without proper handling
Please address these issues before merging, particularly the shell scripting issues and the busy-wait pattern in the dequeuer loop.
PR Bot Information
Version: 1.15.5
- Event Trigger:
issue_comment.created - Correlation ID:
4ee9bd40-c93b-11f0-90b7-69570e81753d
|
@nickytd Command "/InviteCommand" failed with "Review cannot be requested from pull request author.". Additional Information |
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
How to categorize this PR?
/kind enhancement
/area logging
What this PR does / why we need it:
This PR enhances the performance testing framework for the fluent-bit logging plugin and improves disk synchronization in the dque buffering client. It adds support for single namespace performance tests targeting seed clusters, extends the Plutono dashboard with comprehensive metrics visualization, and fixes a typo in the dque client.
Code changes:
isStoopedtoisStoppedin the dque client implementationdque_sizeto track the current queue size and added periodic turbo sync operations (every 30 seconds) to ensure data consistency when turbo mode is enabledSCENARIOparameter supporting both "shoot" (multiple namespaces) and "seed" (single namespace) test modesup.sh→run.shwith scenario-based executiondown.shto support both test scenariosAdditional context:
The enhancements improve observability of the logging pipeline and provide flexibility in performance testing scenarios. The new metrics and dashboard improvements enable better monitoring of the buffer queue health and overall system performance. The seed scenario support allows testing concentrated load patterns that better simulate single-cluster stress conditions.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Please note the extensive changes to the dashboard JSON file - these are primarily visual improvements and metric additions that enhance monitoring capabilities.
Release note: