spanconfig: correctly order updates when applying in the KVSubscriber

[arulajmani]

Previously, there was no ordering guarantee between KVSubscriber events at the same timestamp. As a result, if a batch of events included updates to overlapping spans at the same timestamp, we could apply additions before deletions -- this would cause the additions to get clobbered, which was not the intention. This could lead to missing span configurations, resulting in bugs such as the linked issue.

This patch fixes the issue by sorting deletions before additions if two span configuration events have the same timestamp.

Closes #110908

Release note (bug fix): Previously, altering from a Regional By Row table to a Regional By Table table could cause leaseholders to never move to the databse's primary region. This is now fixed.

[cockroach-teamcity]

This change is 

[andrewbaptist]

Just need to resolve some of the build / lint failures.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)

---

pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go line 455 at r1 (raw file):

		if events[i].Timestamp().Less(events[j].Timestamp()) {
			return true
		} else if events[j].Timestamp().Less(events[i].Timestamp()) {

nit: remove the unnecessary else here.

Also it might be slightly cleaner to call Compare() once and switch rather than calling Less() twice.

[arulajmani]

TFTR! The linter should be happy now.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @andrewbaptist)

---

pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go line 455 at r1 (raw file):

Previously, andrewbaptist (Andrew Baptist) wrote…

nit: remove the unnecessary else here.

Also it might be slightly cleaner to call Compare() once and switch rather than calling Less() twice.

Done.

[andrewbaptist]

Thanks for the updates.

[arulajmani]

TFTR!

bors r=andrewbaptist

[arulajmani]

Looks like the test I introduced is failing under stress -- leaseholders aren't moving quick enough. I'll deflake before merging.

bors r-

[craig]

Canceled.

[arulajmani]

Deflaked and merging. I just bumped the scan interval for this test, to ensure leases end up in the right place quickly.

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 73cb0da to blathers/backport-release-22.2-118001: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.

---

error creating merge commit from 73cb0da to blathers/backport-release-23.1-118001: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.

---

error setting reviewers, but backport branch blathers/backport-release-23.2-118001 is ready: POST https://api.github.com/repos/cockroachdb/cockroach/pulls/118794/requested_reviewers: 422 Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the cockroachdb/cockroach repository. []

Backport to branch 23.2.x failed. See errors above.

---

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