Skip to content

kvserver: fully adopt useReproposalsV2#106750

Merged
craig[bot] merged 18 commits intocockroachdb:masterfrom
tbg:reproposals-v2-burn-in
Jul 19, 2023
Merged

kvserver: fully adopt useReproposalsV2#106750
craig[bot] merged 18 commits intocockroachdb:masterfrom
tbg:reproposals-v2-burn-in

Conversation

@tbg
Copy link
Copy Markdown
Member

@tbg tbg commented Jul 13, 2023

This hard-codes useReproposalsV2 to true (prior to this PR, it's metamorphic with a default of true) and applies the resulting simplifications across the codebase.

These include simplified control flow and an overall simplified architecture.

Closes #105625.

Epic: CRDB-25287
Release note: None

tbg added 4 commits July 13, 2023 10:16
…WithNewLeaseIndexError

These tests don't work with v2 and aren't maintainable or super useful anyway.

We have good randomized coverage of these code paths now via kvnemesis and
TestProposalsWithInjectedLeaseIndexAndReproposalError, and adding better unit
test coverage is tracked in
cockroachdb#106504. In addition to this, it
would be desirable to extend this unit test coverage upwards through the stack
via cockroachdb#105177.
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@tbg tbg force-pushed the reproposals-v2-burn-in branch from 17df203 to 20e033c Compare July 17, 2023 11:30
@tbg tbg marked this pull request as ready for review July 17, 2023 14:20
@tbg tbg requested a review from a team July 17, 2023 14:20
@tbg tbg requested a review from a team as a code owner July 17, 2023 14:20
@tbg tbg requested a review from erikgrinaker July 17, 2023 14:20
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.

LGTM, thanks for the cleanup!

@tbg tbg force-pushed the reproposals-v2-burn-in branch from 93fc06c to d795e6d Compare July 19, 2023 03:52
@tbg
Copy link
Copy Markdown
Member Author

tbg commented Jul 19, 2023

TFTR!

Comments addressed.

bors r=erikgrinaker

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 19, 2023

Build failed:

@tbg
Copy link
Copy Markdown
Member Author

tbg commented Jul 19, 2023

TestRules/join_order flaked, pointed it out here

bors r=erikgrinaker

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 19, 2023

Build succeeded:

@craig craig bot merged commit b2ba2e5 into cockroachdb:master Jul 19, 2023
pav-kv added a commit to pav-kv/cockroach that referenced this pull request Aug 16, 2023
Since cockroachdb#106750, ProposalData is being copied on superseding reproposals. The
caller only knows about the original proposal, and only detaches its context /
tracing span when abandoning the request (e.g. on timeout). By the time of
abandoning the request, there might have been a few superseding reproposals,
and the context / tracing span is being used by multiple ProposalData objects.

In addition to rewriting the original proposal.ctx, we should do the same for
all the ProposalData objects corresponding to the same request. This commit
adds links between consecutive reproposals, and walks the resulting linked list
of proposals when cleaning up / abandoning the request.

Epic: CRDB-25287
Release note: none
pav-kv added a commit to pav-kv/cockroach that referenced this pull request Aug 21, 2023
Since cockroachdb#106750, ProposalData is being copied on superseding reproposals. The
caller only knows about the original proposal, and only detaches its context /
tracing span when abandoning the request (e.g. on timeout). By the time of
abandoning the request, there might have been a few superseding reproposals,
and the context / tracing span is being used by multiple ProposalData objects.

In addition to rewriting the original proposal.ctx, we should do the same for
all the ProposalData objects corresponding to the same request. This commit
unbinds the context for older proposals when reproposing them, and unbinds the
latest reproposal context when cleaning up / abandoning the request.

Epic: CRDB-25287
Release note: none
pav-kv added a commit to pav-kv/cockroach that referenced this pull request Aug 21, 2023
Since cockroachdb#106750, ProposalData is being copied on superseding reproposals. The
caller only knows about the original proposal, and only detaches its context /
tracing span when abandoning the request (e.g. on timeout). By the time of
abandoning the request, there might have been a few superseding reproposals,
and the context / tracing span is being used by multiple ProposalData objects.

In addition to rewriting the original proposal.ctx, we should do the same for
all the ProposalData objects corresponding to the same request. This commit
unbinds the context for older proposals when reproposing them, and unbinds the
latest reproposal context when cleaning up / abandoning the request.

Epic: CRDB-25287
Release note: none
craig bot pushed a commit that referenced this pull request Aug 21, 2023
108483: roachtest: wait smaller duration before timining out lease prefs r=erikgrinaker a=kvoli

Previously, the `lease-preferences` roachtests would fail on the test
tiemout, despite the test actually failing much earlier. 5 minutes
stopping a node, the node is considered dead, and up-replication will
begin. If the cluster doesn't satisfy lease preferences within that 5
minute window, the test will reliably fail -- making the remainder of
the test timeout (30 or 60 minutes), wasted time.

Fail earlier, by adhering to a post event timeout. This timeout set to
10 minutes.

Informs: #108425
Epic: none
Release note: None

108775: kvserver: fix double span Finish on reproposals r=erikgrinaker a=pavelkalinnikov

Since #106750, `ProposalData` is being copied on superseding reproposals. The caller only knows about the original proposal, and only detaches its context / tracing span when abandoning the request (e.g. on timeout). By the time of abandoning the request, there might have been a few superseding reproposals, and the context / tracing span is being used by multiple `ProposalData` objects.

In addition to rewriting the original `proposal.ctx`, we should do the same for all the `ProposalData` objects corresponding to the same request. This commit unbinds the context for older proposals when reproposing them, and unbinds the latest reproposal context when cleaning up / abandoning the request.

Fixes #107521
Fixes #108534
Fixes #108241
Fixes #108663
Fixes #108696
Fixes #108837

Epic: CRDB-25287
Release note: none

108942: backupccl: deflake memory monitor restore test r=rhu713 a=rhu713

Deflake the TestRestoreMemoryMonitoring tests by only asserting the lower bound in the number of files produced in the backup to account for elastic CPU limiter.

Fixes: #108239

Release note: None

109040: server: add more info to the combined statement endpoint r=maryliag a=maryliag

Previously, it was hard to know which tables were
used to populate the SQL Activity, making debug for it complicated.
This commit adds extra information to the
`combinedstmts` to help:
- oldestAggregatedTsReturned
- stmtsSourceTable
- txnsSourceTable

Returning a value for `olderAggregatedTsReturned` will also allow us to show proper warning when the older timestamp doesn't match the start date of the selected time period on the Search Criteria.

Part Of #108989

Release note: None

Co-authored-by: Austen McClernon <austen@cockroachlabs.com>
Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Co-authored-by: Rui Hu <rui@cockroachlabs.com>
Co-authored-by: maryliag <marylia@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kvserver: phase out useReproposalsV2

3 participants