-
Notifications
You must be signed in to change notification settings - Fork 95
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
Issue 4815 - RFE - Add Replication Log Analysis Tool with CLI Support #6466
base: main
Are you sure you want to change the base?
Conversation
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.
Very nice. Few comments
Description: Add a new ReplicationLogAnalyzer class and supporting infrastructure to analyze replication performance across multiple servers. The tool parses replication logs, tracks CSNs, calculates lag times, and generates comprehensive reports in interactive HTML and CSV formats. Based on: https://github.com/droideck/ansible-ds389-repl-monitoring Fixes: 389ds#6465 Reviewed by: ?
c6ec0a1
to
c12c935
Compare
Okay, I think the CLI and lib389 parts are ready for review. I'll do the WebUI part in a separate PR to make the review easier. P.S. if my numpy, plotly, mathplotlib usage is approved, I'll update our ci-image, so the workflow files' changes will be removed from this PR before the merge. |
@@ -40,6 +40,9 @@ jobs: | |||
id: set-matrix | |||
run: echo "matrix=$(python3 .github/scripts/generate_matrix.py ${{ github.event.inputs.pytest_tests }})" >>$GITHUB_OUTPUT | |||
|
|||
- name: Install python3 additional packages |
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.
Is this step really needed ?
maybe we should rather rebuild the docker CI test image with the build prerequisite
@vashirov What do you think ?
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.
Yes, we should add this to our base test image. I think Simon added it here to make the CI happy without changing the image first.
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.
Yes, that's correct. :)
e224d9e
to
563715a
Compare
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.
LGTM
Co-authored-by: Viktor Ashirov <[email protected]>
Description: Add a new ReplicationLogAnalyzer class and supporting infrastructure
to analyze replication performance across multiple servers. The tool parses
replication logs, tracks CSNs, calculates lag times, and generates comprehensive
reports in interactive HTML and CSV formats.
Based on: https://github.com/droideck/ansible-ds389-repl-monitoring
Fixes: #6465
Reviewed by: ?