ci: enable new source reclocking strategy #30031
Draft
+24
−85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
teskje
force-pushed
the
ci-use-latest-reclocking
branch
3 times, most recently
from
a1a680b to
2a4f969
Compare
teskje
force-pushed
the
ci-use-latest-reclocking
branch
4 times, most recently
from
7696bf2 to
b73a85e
Compare
teskje
force-pushed
the
ci-use-latest-reclocking
branch
2 times, most recently
from
7894eb5 to
83d21ab
Compare
5 tasks
teskje
force-pushed
the
ci-use-latest-reclocking
branch
7 times, most recently
from
53517f8 to
ee439b0
Compare
benesch
force-pushed
the
ci-use-latest-reclocking
branch
from
ee439b0 to
9225142
Compare
|
I twiddled the instantiation of the metadata Kafka client. A bit of a hail mary, but maybe it improves something. |
This was referenced Nov 14, 2024
|
I was having trouble understanding which commit was causing which test failures, so I split this out into two separate PRs and triggered nightly on both: |
teskje
force-pushed
the
ci-use-latest-reclocking
branch
2 times, most recently
from
804ae60 to
ab45a65
Compare
teskje
force-pushed
the
ci-use-latest-reclocking
branch
4 times, most recently
from
180363f to
30a8b09
Compare
teskje
force-pushed
the
ci-use-latest-reclocking
branch
2 times, most recently
from
c319c57 to
c7835c5
Compare
teskje
force-pushed
the
ci-use-latest-reclocking
branch
from
c7835c5 to
4dde088
Compare
A bunch of tests set the `kafka_default_metadata_fetch_interval` or the source `TOPIC METADATA REFRESH INTERVAL` to low values. This was done because the default for `kafka_default_metadata_fetch_interval` was 30s and some tests were slowed down by that. The default has now been changed to 1s, so manually lowering the fetch interval is not necessary anymore. Removing the manual setting of the Kafka metadata interval in tests gets rid of some noise. But more importantly, it fixes the issue of some tests setting the interval to a very low value, like '10ms', which now that the metadata fetch interval influences the source tick frequency could negatively impact performance and make tests slower or even time out.
teskje
force-pushed
the
ci-use-latest-reclocking
branch
2 times, most recently
from
14b40f4 to
1293981
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables the new source reclocking scheme ("reclock to latest upper", RLU) in CI, in preparation for switching it on in production too.
Motivation
Part of https://github.com/MaterializeInc/database-issues/issues/7020
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.