Skip to content

kvserver: fix reproposals test with pipelined writes#113658

Merged
craig[bot] merged 4 commits intocockroachdb:masterfrom
pav-kv:reproposal-testing
Nov 3, 2023
Merged

kvserver: fix reproposals test with pipelined writes#113658
craig[bot] merged 4 commits intocockroachdb:masterfrom
pav-kv:reproposal-testing

Conversation

@pav-kv
Copy link
Copy Markdown
Collaborator

@pav-kv pav-kv commented Nov 2, 2023

Before this commit, the test was a no-op reporting 0 metrics. This is due to isOurCommand function which assumed that the key increment request is always the first request in BatchRequest. With pipelined writes, this is not true.

A typical request is:

QueryIntent ["00001-testing",/Min), Increment ["00001-testing",/Min)

In this commit, isOutCommand scans the BatchRequest to find the command.

Before:

observed 0 async write restarts, observed 0/0 injected aborts, 0 injected illegal lease applied indexes
commands reproposed (unchanged): 1
commands reproposed (new LAI): 0

After:

observed 69 async write restarts, observed 0/69 injected aborts, 366 injected illegal lease applied indexes
commands reproposed (unchanged): 1
commands reproposed (new LAI): 297

Fixes #106504
Touches #110551
Epic: none
Release note: none

Epic: none
Release note: none
Before this commit, the test was a no-op reporting 0 metrics. This is
due to isOurCommand function which assumed that the key increment
request is always the first request in BatchRequest. With pipelined
writes, this is not true.

A typical request is:
  QueryIntent ["00001-testing",/Min), Increment ["00001-testing",/Min)

In this commit, isOutCommand scans the BatchRequest to find the command.

Before:
```
observed 0 async write restarts, observed 0/0 injected aborts, 0 injected illegal lease applied indexes
commands reproposed (unchanged): 1
commands reproposed (new LAI): 0
```
After:
```
observed 69 async write restarts, observed 0/69 injected aborts, 366 injected illegal lease applied indexes
commands reproposed (unchanged): 1
commands reproposed (new LAI): 297
```

Epic: none
Release note: none
@pav-kv pav-kv requested review from a team and erikgrinaker November 2, 2023 10:37
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@pav-kv pav-kv added the backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only label Nov 2, 2023
Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, thanks!

Epic: none
Release note: none
@pav-kv
Copy link
Copy Markdown
Collaborator Author

pav-kv commented Nov 3, 2023

bors r=erikgrinaker

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 3, 2023

Build succeeded:

@craig craig bot merged commit 08a253e into cockroachdb:master Nov 3, 2023
@pav-kv pav-kv deleted the reproposal-testing branch November 6, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kvserver: add test to verify tryReproposeWithNewLeaseIndex behaviour

3 participants