-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
backupccl: set shorter closed_timestamp settings in datadriven tests #78489
backupccl: set shorter closed_timestamp settings in datadriven tests #78489
Conversation
The symptom was very similar to #77221. There is a more heavy handed solution here where we reject datadriven tests from running Stressed for 15mins and not failing, previously it would fail within a minute. |
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.
This seems reasonable to address the flake.
It does make me wonder how many tests would be subtly wrong if run in multi-node configurations:
> rg '^SET CLUSTER SETTING' | grep "/testdata/" | wc -l
252
``
TFTR! bors r=stevendanna |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
bors r=stevendanna |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
bors r=stevendanna |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
bors r=stevendanna |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
1788529
to
5a0360d
Compare
bazel failure is an unrelated flake bors r=stevendanna |
bors r=stevendanna |
78489: backupccl: set shorter closed_timestamp settings in datadriven tests r=stevendanna a=adityamaru This change drops the `kv.closed_timestamp.side_transport_interval` and `kv.closed_timestamp.target_duration` in the servers started as part of the datadriven framework. This change was motivated by the fact that cluster settings set in certain multi-node datadriven tests were not being observed by all nodes in the cluster. Cluster settings are now backed by rangefeeds instead of gossiping the SystemSpanConfig and so these settings should speed up when all nodes become aware of changes to their values. Fixes: #77264 Release note: None Co-authored-by: Aditya Maru <[email protected]>
Build failed: |
bors r+ |
Build failed: |
This change drops the `kv.closed_timestamp.side_transport_interval` and `kv.closed_timestamp.target_duration` in the servers started as part of the datadriven framework. This change was motivated by the fact that cluster settings set in certain multi-node datadriven tests were not being observed by all nodes in the cluster. Cluster settings are now backed by rangefeeds instead of gossiping the SystemSpanConfig and so these settings should speed up when all nodes become aware of changes to their values. Fixes: cockroachdb#77264 Release note: None
5a0360d
to
38fd057
Compare
Flakes again 😢 Going to see if bors lets me through. bors r=stevendanna |
Build succeeded: |
This change drops the
kv.closed_timestamp.side_transport_interval
and
kv.closed_timestamp.target_duration
in the servers started as partof the datadriven framework. This change was motivated by the fact that
cluster settings set in certain multi-node datadriven tests were not being
observed by all nodes in the cluster. Cluster settings are now backed by
rangefeeds instead of gossiping the SystemSpanConfig and so these settings
should speed up when all nodes become aware of changes to their values.
Fixes: #77264
Release note: None