kvserver: use narrow checkpoints in consistency checker #95963

pav-kv · 2023-01-26T10:24:38Z

This commit modifies the consistency checker to save partial checkpoints
instead of full ones. The partial checkpoints contain the data for the
inconsistent range and its immediate left and right neighbour in the Store.
Only the inconsistent range's Replica is locked at the time of snapshotting the
store before creating the checkpoint, other ranges can be out of sync. They are
checkpointed too, to give more context when e.g. debugging non-trivial
split/merge scenarios.

Release note (ops change): In the rare event of Range inconsistency, the
consistency checker saves a storage checkpoint on each storage the Range
belongs to. Previously, this was a full checkpoint, so its cost could quickly
escalate on the nodes that went on running. This change makes the checkpoints
partial, i.e. they now only contain the relevant range and its neighbours. This
eliminates the time pressure on the cluster operator to remove the checkpoints.

Resolves #90543
Epic: none

blathers-crl · 2023-01-26T10:24:43Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

cockroach-teamcity · 2023-01-26T10:24:52Z

This change is

pkg/kv/kvserver/store.go

erikgrinaker

Would be worth having an integration test for this.

pkg/kv/kvserver/replica_consistency.go

pkg/kv/kvserver/store.go

pkg/kv/kvserver/replica_consistency.go

erikgrinaker · 2023-01-26T13:44:14Z

PPS: We should test that we can open the narrow checkpoint just like any other Pebble database as well, e.g. by running various cockroach debug commands on it.

RaduBerinde · 2023-01-26T15:17:05Z

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

LookupPrecedingReplica finds a Replica, whereas the VisitKeyRange with the visitor written here finds a Replica or a placeholder. We could unify this so that both left and right are Replica, or both are Replica/placeholder. Which way makes more sense?

We don't care about placeholders, I think. We can e.g. add a corresponding LookupNextReplica with the same behavior.

To guard against checkpointing too much, we can provide 3 CheckpointSpans below, instead of a single wider one. The risk with one wider span is that the neighbouring ranges are not locked, so we may end up touching more ranges than intended.

Yeah, this can be pathological in certain extreme cases, e.g. where we recently removed a bunch of replicas but they haven't been compacted all the way down yet. But I suppose there's the same risk if we just merged a bunch of ranges into the inconsistent range.

I think we probably want to checkpoint the entire span between the neighbouring replicas, because any recently-removed ranges could be relevant to the inconsistency, and we'll just have to take the risk that they may contain more than 3 ranges worth of data. In most cases they wouldn't, and in any case it's a strict improvement over the current state -- we can reconsider if we do find any issues with it later.

Another vote for a single span over the ranges. Note that the narrow checkpoints only exclude SST files that are fully outside of the spans; they do not "cut up" SST files that intersect the spans (and they don't trim WALs). So if we have ranges [a c), [f h), we may still see keys like d. That might become confusing when we use this for debugging. We should take one continuous range that contains the areas of interest.

The exception of course would be if we want to also include something from the very beginning of the keyspace (the meta ranges?).

erikgrinaker

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @pavelkalinnikov, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

The exception of course would be if we want to also include something from the very beginning of the keyspace (the meta ranges?).

The meta ranges likely won't be on this node in larger clusters, and it isn't clear that they'd be helpful anyway. But I do think we want to get the range-local keyspans, both replicated and unreplicated, and those will have to be separate spans since they're prefixed by range ID.

This might be confusing, as your say, since we'll see various parts of the surrounding ranges at various LSM levels, but I think in almost all cases we'll only be interested in the user keyspan anyway.

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @jbowens, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

The exception of course would be if we want to also include something from the very beginning of the keyspace (the meta ranges?).

The meta ranges likely won't be on this node in larger clusters, and it isn't clear that they'd be helpful anyway. But I do think we want to get the range-local keyspans, both replicated and unreplicated, and those will have to be separate spans since they're prefixed by range ID.

This might be confusing, as your say, since we'll see various parts of the surrounding ranges at various LSM levels, but I think in almost all cases we'll only be interested in the user keyspan anyway.

I'm also inclined to make it a single span. One complication here is that one range already consists of multiple spans. See rditer.makeAllKeySpans, I counted 6 spans. Now, if we have 3 ranges 6 spans each, we can either use the 18 spans as they are, or merge them into 6 wide spans. I'm in favour of the latter approach, but it looks like we would need cross-rangeID span helpers for that.

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…

I'm also inclined to make it a single span. One complication here is that one range already consists of multiple spans. See rditer.makeAllKeySpans, I counted 6 spans. Now, if we have 3 ranges 6 spans each, we can either use the 18 spans as they are, or merge them into 6 wide spans. I'm in favour of the latter approach, but it looks like we would need cross-rangeID span helpers for that.

@tbg Do you think we could adapt rditer/select.go to work across range IDs?

Right now we have:

func Select(rangeID roachpb.RangeID, opts SelectOpts) []roachpb.Span

But it seems easy to extend it to:

func SelectMulti(begin, end roachpb.RangeID, opts SelectOpts) []roachpb.Span

It would use the starting points of the begin range, and end points of the end range. A little complication is that the caller must correctly compute the user keys span in opts. For a single range it's easy: take the range descriptor's span; for a multi-range case the caller should take the first key of the left range, and the end key of the last range.

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…

@tbg Do you think we could adapt rditer/select.go to work across range IDs?

Right now we have:
func Select(rangeID roachpb.RangeID, opts SelectOpts) []roachpb.Span
But it seems easy to extend it to:
func SelectMulti(begin, end roachpb.RangeID, opts SelectOpts) []roachpb.Span
It would use the starting points of the begin range, and end points of the end range. A little complication is that the caller must correctly compute the user keys span in opts. For a single range it's easy: take the range descriptor's span; for a multi-range case the caller should take the first key of the left range, and the end key of the last range.

My mistake, I think we can't assume that consecutive ranges according to the key are consecutive w.r.t. range IDs. So, for all rangeID-based keys, we need to take all the spans as is. It's only the user key spans that we can (and should) merge.

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…

My mistake, I think we can't assume that consecutive ranges according to the key are consecutive w.r.t. range IDs. So, for all rangeID-based keys, we need to take all the spans as is. It's only the user key spans that we can (and should) merge.

Hm, but then we're limited to only finding bugs that affect the user keyspace. What if the bug is in the rangeID-based space? Wouldn't we want the LSMs touching neighbouring range IDs included as well?

@erikgrinaker Do you remember which part of the keyspace contained that Pebble bug? Could it manifest itself in other parts?

erikgrinaker

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @pavelkalinnikov, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

My mistake, I think we can't assume that consecutive ranges according to the key are consecutive w.r.t. range IDs. So, for all rangeID-based keys, we need to take all the spans as is. It's only the user key spans that we can (and should) merge.

Correct, they'll often not have consecutive range IDs.

What if the bug is in the rangeID-based space? Wouldn't we want the LSMs touching neighbouring range IDs included as well?

Yeah, may as well grab it just in case. For the range-local keyspace let's extend the spans to the neighbouring ranges by range ID, while for the user keyspace we'll extend it to the neighbouring ranges by key.

One could argue that we should also grab the range-local spans for the neighbouring ranges by user key, but I don't think we need to overcomplicate this. I think what we mostly care about is the inconsistent range itself and the surrounding LSM contents, and so let's just grab that.

Do you remember which part of the keyspace contained that Pebble bug? Could it manifest itself in other parts?

The user keyspace.

It could manifest in the range-local keyspace too, since we do use Pebble range tombstones there, but I think they're much less likely to span ranges given the structure of the keyspace there (it typically has many different prefixes for different types of data).

tbg

There's a roachtest/inconsistency roachtest that we might want to spruce up a bit.

Reviewed 5 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @jbowens, @pavelkalinnikov, and @RaduBerinde)

pkg/kv/kvserver/replica_consistency.go line 778 at r2 (raw file):

compacted, which leads to checkpoints becoming a full copy of a past state. This
is not time critical because the checkpoints are localized to a single / few
range(s), and over time may grow to consume only O(GB) disk space. There is no

The range size is configurable, and also we're starting to talk about larger range sizes. You could make this a bit more general and say that it's O(RangeSize).

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

My mistake, I think we can't assume that consecutive ranges according to the key are consecutive w.r.t. range IDs. So, for all rangeID-based keys, we need to take all the spans as is. It's only the user key spans that we can (and should) merge.

Correct, they'll often not have consecutive range IDs.

What if the bug is in the rangeID-based space? Wouldn't we want the LSMs touching neighbouring range IDs included as well?

Yeah, may as well grab it just in case. For the range-local keyspace let's extend the spans to the neighbouring ranges by range ID, while for the user keyspace we'll extend it to the neighbouring ranges by key.

One could argue that we should also grab the range-local spans for the neighbouring ranges by user key, but I don't think we need to overcomplicate this. I think what we mostly care about is the inconsistent range itself and the surrounding LSM contents, and so let's just grab that.

Do you remember which part of the keyspace contained that Pebble bug? Could it manifest itself in other parts?

The user keyspace.

It could manifest in the range-local keyspace too, since we do use Pebble range tombstones there, but I think they're much less likely to span ranges given the structure of the keyspace there (it typically has many different prefixes for different types of data).

Determining a single RSpan over which to checkpoint the key-prefixed data makes sense to me. Space that has a placeholder on it ought to be empty anyway. So I think we should:

find left and right neighboring range, note the left start key, the right end key, and both range ids
rspan=[left start key, right end key)
spans = append(rditer.Select(rangeID, ReplicatedBySpan:rspan, ReplicatedByRangeID:true), rditer.Select(leftRangeID, ReplicatedByRangeID:true), rditer.Select(rightRangeID, ReplicatedByRangeID:true)

In words, we pretend that our range has the wider key span and checkpoint that (this is a couple of spans under the hood due to local-key and local-rangeid addressed keys), but in addition include the two replicated rangeid-based key spans for the left and right neighbor.

For debugging purposes we should also throw in the rangeid-unreplicated key spans, i.e. set the corresponding bool to true in spans = above for all Select invocations.

pkg/kv/kvserver/store.go line 2911 at r2 (raw file):

		spans = append(spans, span)
	}
	if err := s.engine.CreateCheckpoint(checkpointDir, spans); err != nil {

It would be good to mark the checkpoint as partial and perhaps encode (in a text file?) what the start and end key and rangeIDs were that are guaranteed to be covered.

erikgrinaker

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @pavelkalinnikov, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, tbg (Tobias Grieger) wrote…

Determining a single RSpan over which to checkpoint the key-prefixed data makes sense to me. Space that has a placeholder on it ought to be empty anyway. So I think we should:

find left and right neighboring range, note the left start key, the right end key, and both range ids

rspan=[left start key, right end key)

spans = append(rditer.Select(rangeID, ReplicatedBySpan:rspan, ReplicatedByRangeID:true), rditer.Select(leftRangeID, ReplicatedByRangeID:true), rditer.Select(rightRangeID, ReplicatedByRangeID:true)

In words, we pretend that our range has the wider key span and checkpoint that (this is a couple of spans under the hood due to local-key and local-rangeid addressed keys), but in addition include the two replicated rangeid-based key spans for the left and right neighbor.

For debugging purposes we should also throw in the rangeid-unreplicated key spans, i.e. set the corresponding bool to true in spans = above for all Select invocations.

Yep. Note that the rangeID neighbours will often be different ranges than the user key neighbours. We don't want to conflate them, since we could otherwise checkpoint the entire range-local span, which can be pretty large. In other words, if we have an inconsistent range with the following surrounding ranges on that node:

[a-b): range 8
[d-f): range 5 (inconsistent)
[k-m): range 2

The node has replicas for ranges 2, 3, 5, 6, 8.

Then we want to checkpoint the user keyspan [a-m), and the range-local keyspans for ranges [3-6].

erikgrinaker

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jbowens, @pavelkalinnikov, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

Yep. Note that the rangeID neighbours will often be different ranges than the user key neighbours. We don't want to conflate them, since we could otherwise checkpoint the entire range-local span, which can be pretty large. In other words, if we have an inconsistent range with the following surrounding ranges on that node:

[a-b): range 8

[d-f): range 5 (inconsistent)

[k-m): range 2

The node has replicas for ranges 2, 3, 5, 6, 8.

Then we want to checkpoint the user keyspan [a-m), and the range-local keyspans for ranges [3-6].

Taking into account whether the local keyspans are prefixed by key or range ID, of course.

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @jbowens, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

Taking into account whether the local keyspans are prefixed by key or range ID, of course.

SGTM, thanks for the suggestions. WIP.

pkg/kv/kvserver/store.go line 2911 at r2 (raw file):

Previously, tbg (Tobias Grieger) wrote…

It would be good to mark the checkpoint as partial and perhaps encode (in a text file?) what the start and end key and rangeIDs were that are guaranteed to be covered.

I agree. I was thinking at least including this information in the log/prevent-startup-file message. Do you think it should a separate file though?

jbowens

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @pavelkalinnikov, @RaduBerinde, and @tbg)

pkg/kv/kvserver/store.go line 2911 at r2 (raw file):

Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…

I agree. I was thinking at least including this information in the log/prevent-startup-file message. Do you think it should a separate file though?

+1 to encoding the spans somewhere: the Pebble checkpoint won't encode any information about which spans were checkpointed, and it won't be possible to tell which regions of the keyspace should be consistent. I could imagine looking at one of these partial checkpoints and getting confused because there exist keys outside the checkpointed spans that are inconsistent.

RaduBerinde · 2023-01-27T19:30:19Z

pkg/kv/kvserver/store.go line 2911 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

+1 to encoding the spans somewhere: the Pebble checkpoint won't encode any information about which spans were checkpointed, and it won't be possible to tell which regions of the keyspace should be consistent. I could imagine looking at one of these partial checkpoints and getting confused because there exist keys outside the checkpointed spans that are inconsistent.

I think it would be a good idea to include this information in the checkpoint's manifest.. And have any tools you use warn you about it if these are set.. I'd need to update the manifest format though.

RaduBerinde · 2023-01-27T19:41:01Z

pkg/kv/kvserver/store.go line 2911 at r2 (raw file):

Previously, RaduBerinde wrote…

I think it would be a good idea to include this information in the checkpoint's manifest.. And have any tools you use warn you about it if these are set.. I'd need to update the manifest format though.

Filed cockroachdb/pebble#2285

pav-kv

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @herkolategan, @srosenberg, and @tbg)

pkg/kv/kvserver/store.go line 2892 at r1 (raw file):

Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…

Right, this isn't the final version, I just did the easy by-key part for now, and testing things. I will add by-rangeID neighbours shortly.

Added the rangeID neighbours too.

pav-kv · 2023-01-31T20:49:49Z

@erikgrinaker @tbg This is ready for another round. I recommend looking commit by commit.

pav-kv · 2023-01-31T20:53:59Z

pkg/kv/kvserver/store.go

+	if left != nil {
+		userKeys.Key = left.Desc().StartKey
+		if id := left.RangeID; id < prevID || id > nextID {
+			spans = append(spans, rditer.Select(left.RangeID, byRangeID)...)


s/left.RangeID/id/

pav-kv · 2023-01-31T20:54:06Z

pkg/kv/kvserver/store.go

+	if right != nil {
+		userKeys.EndKey = right.Desc().EndKey
+		if id := right.RangeID; id < prevID || id > nextID {
+			spans = append(spans, rditer.Select(right.RangeID, byRangeID)...)


s/right.RangeID/id/

tbg

with exception of the case in which we have neighboring rangeIDs but no neighboring inited ranges. It sounds like we're not collecting the neighboring rangeID spans in the checkpoint then. Either explain it in a comment or fix it, depending on it's broken. After that good to merge.

Do we have coverage that the checkpoint contains ... the right thing? If we passed the wrong spans to pebble or pebble would just create an empty checkpoint, would we find out anywhere? If not, let's add... some testing at least. Unit testing of checkpointSpans would seem appropriate for example, or we do some additional verification in TestCheckConsistencyInconsistent?

Reviewed 1 of 5 files at r3, 1 of 3 files at r4, 5 of 5 files at r6, 2 of 2 files at r7, 1 of 1 files at r8, 1 of 1 files at r9, all commit messages.
Dismissed @RaduBerinde and @jbowens from a discussion.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker, @herkolategan, @pavelkalinnikov, and @srosenberg)

pkg/cmd/roachtest/tests/inconsistency.go line 145 at r8 (raw file):

		// multiple if multiple consistency checks fail in close succession), and
		// puts spans information into the checkpoint.txt file in it.
		c.Run(ctx, c.Node(n), "find {store-dir}/auxiliary/checkpoints/r1_at_*/checkpoint.txt")

Heh, I thought this was a bogus invocation because the * looks like it would get shell expanded, but I think I added proper escaping here at some point. Still, this would look less weird as

find {store-dir}/auxiliary/checkpoints -name 'checkpoint.txt'

I cede the loss of coverage of the r1_ match.

Also, this would be the other approach:

		det, err := c.RunWithDetailsSingleNode()
		det.Stdout <-- has the path and can be validated/used for the subsequend cmds

I'm fine with the current code as-is just as long as it works.

pkg/kv/kvserver/store.go line 2923 at r6 (raw file):

	if left != nil {
		userKeys.Key = left.Desc().StartKey
		if id := left.RangeID; id < prevID || id > nextID {

So if we don't hit this condition, we don't collect the neighboring rangeID spans? Ditto for the right branch.

pkg/kv/kvserver/store_replica_btree_test.go line 126 at r9 (raw file):

		{"a", nil, repl2},
		{"aa", nil, repl3},
		{"b", repl2, repl3},

Nice, thanks for adding the test.

pav-kv

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker, @herkolategan, and @srosenberg)

pkg/kv/kvserver/store.go line 2923 at r6 (raw file):

Previously, tbg (Tobias Grieger) wrote…

So if we don't hit this condition, we don't collect the neighboring rangeID spans? Ditto for the right branch.

We do collect them above where we take by-rangeID spans covering all of [prevID, nextID]. This check here skips adding spans for an ID that is in the already covered [prevID, nextID].

pav-kv

Yep, a unit-test for checkpointSpans would be nicest, I think. Will do.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker, @herkolategan, and @srosenberg)

pav-kv

Add TestStoreCheckpointSpans. PTAL.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @erikgrinaker, @herkolategan, @srosenberg, and @tbg)

pav-kv · 2023-02-06T14:14:18Z

@erikgrinaker wanna take a look at which spans we checkpoint before I merge? See Store.checkpointSpans function and test.

erikgrinaker · 2023-02-06T14:33:14Z

Yeah, I'll have a look.

tbg

Reviewed 6 of 6 files at r10, 2 of 2 files at r11, 1 of 1 files at r12, 1 of 1 files at r13, 1 of 1 files at r14, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @erikgrinaker, @herkolategan, and @srosenberg)

erikgrinaker

LGTM, thanks!

pkg/kv/kvserver/store.go

This commit modifies the consistency checker to save partial checkpoints instead of full ones. The partial checkpoints contain the data for the inconsistent range and its immediate left and right neighbour in the Store. Only the inconsistent range's Replica is locked at the time of snapshotting the store before creating the checkpoint, other ranges can be out of sync. They are checkpointed too, to give more context when e.g. debugging non-trivial split/merge scenarios. Release note (ops change): In the rare event of Range inconsistency, the consistency checker saves a storage checkpoint on each storage the Range belongs to. Previously, this was a full checkpoint, so its cost could quickly escalate on the nodes that went on running. This change makes the checkpoints partial, i.e. they now only contain the relevant range and its neighbours. This eliminates the time pressure on the cluster operator to remove the checkpoints. Epic: none

Release note: none Epic: none

This commit modifies the consistency check failure roachtest to verify that all the nodes of an inconsistent range create a checkpoint that can be opened with tooling. Release note: none Epic: none

Release note: none Epic: none

pav-kv · 2023-02-06T16:27:42Z

bors r=tbg,erikgrinaker

craig · 2023-02-06T17:47:57Z

Build succeeded:

Bazel Essential CI (Cockroach)

pav-kv requested review from erikgrinaker and tbg January 26, 2023 10:24

pav-kv requested a review from a team as a code owner January 26, 2023 10:24

pav-kv requested a review from a team January 26, 2023 10:24

pav-kv requested review from a team as code owners January 26, 2023 10:24

pav-kv requested a review from jbowens January 26, 2023 10:24

pav-kv force-pushed the use-narrow-checkpoint branch from f29bbc7 to 6daea4d Compare January 26, 2023 10:26

pav-kv commented Jan 26, 2023

View reviewed changes

pkg/kv/kvserver/store.go Outdated Show resolved Hide resolved

pav-kv force-pushed the use-narrow-checkpoint branch from 6daea4d to 16374ce Compare January 26, 2023 10:45

erikgrinaker reviewed Jan 26, 2023

View reviewed changes

pkg/kv/kvserver/replica_consistency.go Outdated Show resolved Hide resolved

pkg/kv/kvserver/store.go Outdated Show resolved Hide resolved

pkg/kv/kvserver/store.go Outdated Show resolved Hide resolved

pkg/kv/kvserver/store.go Outdated Show resolved Hide resolved

erikgrinaker reviewed Jan 26, 2023

View reviewed changes

pkg/kv/kvserver/replica_consistency.go Outdated Show resolved Hide resolved

erikgrinaker reviewed Jan 26, 2023

View reviewed changes

pav-kv commented Jan 26, 2023

View reviewed changes

erikgrinaker reviewed Jan 26, 2023

View reviewed changes

tbg reviewed Jan 27, 2023

View reviewed changes

erikgrinaker reviewed Jan 27, 2023

View reviewed changes

pav-kv commented Jan 27, 2023

View reviewed changes

jbowens reviewed Jan 27, 2023

View reviewed changes

pav-kv force-pushed the use-narrow-checkpoint branch from 16374ce to b3c64a0 Compare January 30, 2023 10:35

pav-kv force-pushed the use-narrow-checkpoint branch from 5c857c6 to cf35c11 Compare January 31, 2023 17:48

pav-kv commented Jan 31, 2023

View reviewed changes

pav-kv force-pushed the use-narrow-checkpoint branch 2 times, most recently from bbfe4aa to 198090b Compare January 31, 2023 20:48

pav-kv requested review from erikgrinaker and tbg January 31, 2023 20:49

pav-kv commented Jan 31, 2023

View reviewed changes

tbg approved these changes Feb 2, 2023

View reviewed changes

pav-kv commented Feb 6, 2023

View reviewed changes

pav-kv requested a review from tbg February 6, 2023 12:52

pav-kv force-pushed the use-narrow-checkpoint branch from f2b87e7 to f1257f1 Compare February 6, 2023 13:08

tbg approved these changes Feb 6, 2023

View reviewed changes

erikgrinaker approved these changes Feb 6, 2023

View reviewed changes

pkg/kv/kvserver/store.go Outdated Show resolved Hide resolved

pav-kv added 5 commits February 6, 2023 16:22

roachtest,kvserver: brush up consistency check tests

87e8de9

Release note: none Epic: none

roachtest: inspect checkpoint in consistency check test

690cebe

This commit modifies the consistency check failure roachtest to verify that all the nodes of an inconsistent range create a checkpoint that can be opened with tooling. Release note: none Epic: none

kvserver: add LookupNextReplica test

88f4f51

Release note: none Epic: none

kvserver: test Store.checkpointSpans

c87ca2d

Release note: none Epic: none

pav-kv force-pushed the use-narrow-checkpoint branch from f1257f1 to c87ca2d Compare February 6, 2023 16:22

craig bot merged commit b031933 into cockroachdb:master Feb 6, 2023

pav-kv deleted the use-narrow-checkpoint branch February 6, 2023 18:11

cockroach-teamcity mentioned this pull request Feb 7, 2023

PR #95963 - kvserver: use narrow checkpoints in consistency checker cockroachdb/docs#16198

Open

erikgrinaker mentioned this pull request Feb 7, 2023

roachtest: inconsistency failed #96707

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kvserver: use narrow checkpoints in consistency checker #95963

kvserver: use narrow checkpoints in consistency checker #95963

pav-kv commented Jan 26, 2023 •

edited

Loading

blathers-crl bot commented Jan 26, 2023

cockroach-teamcity commented Jan 26, 2023

erikgrinaker left a comment

erikgrinaker commented Jan 26, 2023

RaduBerinde commented Jan 26, 2023

erikgrinaker left a comment

pav-kv left a comment

pav-kv left a comment

pav-kv left a comment

pav-kv left a comment

erikgrinaker left a comment

tbg left a comment

erikgrinaker left a comment

erikgrinaker left a comment

pav-kv left a comment

jbowens left a comment

RaduBerinde commented Jan 27, 2023

RaduBerinde commented Jan 27, 2023

pav-kv left a comment

pav-kv commented Jan 31, 2023 •

edited

Loading

pav-kv Jan 31, 2023

pav-kv Jan 31, 2023

tbg left a comment

pav-kv left a comment

pav-kv left a comment

pav-kv left a comment

pav-kv commented Feb 6, 2023

erikgrinaker commented Feb 6, 2023

tbg left a comment

erikgrinaker left a comment

pav-kv commented Feb 6, 2023

craig bot commented Feb 6, 2023

kvserver: use narrow checkpoints in consistency checker #95963

kvserver: use narrow checkpoints in consistency checker #95963

Conversation

pav-kv commented Jan 26, 2023 • edited Loading

blathers-crl bot commented Jan 26, 2023

cockroach-teamcity commented Jan 26, 2023

erikgrinaker left a comment

Choose a reason for hiding this comment

erikgrinaker commented Jan 26, 2023

RaduBerinde commented Jan 26, 2023

erikgrinaker left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

erikgrinaker left a comment

Choose a reason for hiding this comment

tbg left a comment

Choose a reason for hiding this comment

erikgrinaker left a comment

Choose a reason for hiding this comment

erikgrinaker left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

jbowens left a comment

Choose a reason for hiding this comment

RaduBerinde commented Jan 27, 2023

RaduBerinde commented Jan 27, 2023

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv commented Jan 31, 2023 • edited Loading

pav-kv Jan 31, 2023

Choose a reason for hiding this comment

pav-kv Jan 31, 2023

Choose a reason for hiding this comment

tbg left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv left a comment

Choose a reason for hiding this comment

pav-kv commented Feb 6, 2023

erikgrinaker commented Feb 6, 2023

tbg left a comment

Choose a reason for hiding this comment

erikgrinaker left a comment

Choose a reason for hiding this comment

pav-kv commented Feb 6, 2023

craig bot commented Feb 6, 2023

pav-kv commented Jan 26, 2023 •

edited

Loading

pav-kv commented Jan 31, 2023 •

edited

Loading