closedts: bump kv.closed_timestamp.target_duration default #31837
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
|
This change is |
bdarnell
approved these changes
Oct 24, 2018
|
bors r+ |
|
bors r- |
Canceled |
This change bumps the closed timestamp target duration to 30s. We're not using the timestamp except in special cases in 2.1, so it doesn't make sense for it to be so low. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/ctDefault
branch
from
010dc56
to
e2ea965
Compare
|
Ok, fixed the lint error and waited for TC. bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Oct 24, 2018
31829: util: Send only last metric value to Graphite r=nvanbenschoten a=neeral Cockroach was emitting a ridiculously large number of metrics per minute because an instance of PrometheusExporter was being reused without clearing it after every iteration. Release note: Fix bug in graphite metrics sender where cockroach was collecting and sending all data points since startup instead of only the latest data point. 31837: closedts: bump kv.closed_timestamp.target_duration default r=nvanbenschoten a=nvanbenschoten This change bumps the closed timestamp target duration to 30s. We're not using closed timestamps, except in special cases in 2.1, so it doesn't make sense for the setting to be so low if it risks pushing live transactions. Release note: None Co-authored-by: neeral <neeral@users.noreply.github.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Build succeeded |
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Dec 18, 2018
…efeed Before this PR, ranges which emitted min lease applied indices of zero were ignored. This happened because the logic to track and store MLAIs takes care to not allow storing indices less than the currently stored value. Go's maps return zero values when queried for non-existing keys and because it is not the case that 0 < 0, we never added zero entries to the map. This PR was motivated by the formerly unstable cdc/rangefeed roachtest. The author has verified that the formerly flakey cdc rangefeed roachtest now reliably passes after making minor changes to the test. The change updates the target steady latency from 1m to 2m to adjust for the increasing of kv.closedts.target_duration from 5s to 30s (see cockroachdb#31837). It also adds a short sleep between installing and running the TPCC workload which seems to deflake some errors which @Danhz had observed. Lastly, it increase the replica_rangefeed channel buffer size from 512 to 4096 to mitigate errors obserted related to "buffer capacity exceeded due to slow consumer". Release note: None
craig bot
pushed a commit
that referenced
this pull request
Dec 18, 2018
33243: closedts: fix bug ignoring zero MLIAs to stabilize roachtest cdc/rangedfeed r=ajwerner a=ajwerner Before this PR, ranges which emitted min lease applied indices of zero were ignored. This happened because the logic to track and store MLAIs takes care to not allow storing indices less than the currently stored value. Go's maps return zero values when queried for non-existing keys and because it is not the case that 0 < 0, we never added zero entries to the map. This PR was motivated by the formerly unstable cdc/rangefeed roachtest. The author has verified that the formerly flakey cdc rangefeed roachtest now reliably passes after making minor changes to the test. The change updates the target steady latency from 1m to 2m to adjust for the increasing of kv.closedts.target_duration from 5s to 30s (see #31837). It also adds a short sleep between installing and running the TPCC workload which seems to deflake some errors which @danhhz had observed. Lastly, it increase the replica_rangefeed channel buffer size from 512 to 4096 to mitigate errors obserted related to "buffer capacity exceeded due to slow consumer". Release note: None Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
changangela
pushed a commit
to changangela/cockroach
that referenced
this pull request
Dec 19, 2018
…efeed Before this PR, ranges which emitted min lease applied indices of zero were ignored. This happened because the logic to track and store MLAIs takes care to not allow storing indices less than the currently stored value. Go's maps return zero values when queried for non-existing keys and because it is not the case that 0 < 0, we never added zero entries to the map. This PR was motivated by the formerly unstable cdc/rangefeed roachtest. The author has verified that the formerly flakey cdc rangefeed roachtest now reliably passes after making minor changes to the test. The change updates the target steady latency from 1m to 2m to adjust for the increasing of kv.closedts.target_duration from 5s to 30s (see cockroachdb#31837). It also adds a short sleep between installing and running the TPCC workload which seems to deflake some errors which @Danhz had observed. Lastly, it increase the replica_rangefeed channel buffer size from 512 to 4096 to mitigate errors obserted related to "buffer capacity exceeded due to slow consumer". Release note: None
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 change bumps the closed timestamp target duration to 30s. We're not
using closed timestamps, except in special cases in 2.1, so it doesn't make
sense for the setting to be so low if it risks pushing live transactions.
Release note: None