-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Online DDL: do not update last_throttled_timestamp with zero value #16395
Online DDL: do not update last_throttled_timestamp with zero value #16395
Conversation
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
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.
It might be a good idea to initialize one of the OnlineDDL tests with the strict modes that Vitess might run with.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16395 +/- ##
==========================================
- Coverage 68.68% 68.67% -0.02%
==========================================
Files 1548 1548
Lines 199084 199085 +1
==========================================
- Hits 136736 136712 -24
- Misses 62348 62373 +25 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Good idea, done. |
(they ought to have such |
Description
_vt.schema_migrations.last_throttled_timestamp
indicates the last time a vreplication migration was throttled, value copied from_vt.vreplication.time_throttled
. However, it is possible for the value to be zero iftimeThrottled
is uninitialized. In this PR we avoid updatinglast_throttled_timestamp
to the zero value.Related Issue(s)
Fixes #16394
Checklist
Deployment Notes