-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[SDBM-2310] Extend Mysql replication metric reporting to support mixed replication topologies #22485
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
base: master
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51485de760
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| has_traditional_replication = replica_stats or replicas_connected.get('Replicas_connected', 0) > 0 | ||
| if has_traditional_replication: | ||
| results.update(replica_stats) | ||
| results.update(replicas_connected) | ||
| return REPLICA_VARS |
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.
Preserve replication status when source has 0 replicas
When replication_enabled is true on a traditional source with no connected replicas (or all replicas temporarily offline), replica_stats is empty and Replicas_connected is 0, so has_traditional_replication is false and the method returns {}. That prevents _check_replication_status from running and suppresses the mysql.replication.* service checks/metrics that previously signaled a WARNING for “source with 0 replicas,” which can mask replica-loss conditions in non-group replication setups.
Useful? React with 👍 / 👎.
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: 76e58bf | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
What does this PR do?
Currently when replication metric reporting is enabled we report back group replication OR traditional primary/replica replication. It's possible for Mysql topologies to make use of both replication methods. The changes in this pull request extend support so that we collect both if they are both identified as active.
A new test harness for integration tests was added which spins up a 3 node group replication setup with a 4th instance that is configured as a traditional replication replica of node1.
Motivation
The ability to collect metrics for mixed replication topologies was requested in https://datadoghq.atlassian.net/browse/SDBM-2310
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged