Skip to content

rac2,replica_rac2: add ForceFlushIndexChangedLocked to Processor, Ran…#136321

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
sumeerbhola:rac2_ffi
Dec 2, 2024
Merged

rac2,replica_rac2: add ForceFlushIndexChangedLocked to Processor, Ran…#136321
craig[bot] merged 1 commit intocockroachdb:masterfrom
sumeerbhola:rac2_ffi

Conversation

@sumeerbhola
Copy link
Collaborator

…geController

This is used to set the highest index up to which all send-queues in pull mode must be force-flushed.

Informs #135601

Epic: CRDB-37515

Release note: None

@sumeerbhola sumeerbhola requested review from kvoli and pav-kv November 27, 2024 21:35
@sumeerbhola sumeerbhola requested a review from a team as a code owner November 27, 2024 21:35
@blathers-crl
Copy link

blathers-crl bot commented Nov 27, 2024

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

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

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @pav-kv and @sumeerbhola)


pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go line 612 at r1 (raw file):

// ForceFlushIndexChangedLocked implements Processor.
func (p *processorImpl) ForceFlushIndexChangedLocked(ctx context.Context, index uint64) {

Also here, we could add an assertion on the respective mutexes.


pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go line 1534 at r1 (raw file):

// ForceFlushIndexChangedLocked implements RangeController.
func (rc *rangeController) ForceFlushIndexChangedLocked(ctx context.Context, index uint64) {

We could assert on raftMu and replMu being held here as well.


pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go line 2571 at r1 (raw file):

		// needed to rely on this force-flush until the send-queue was empty. That
		// knowledge will become known in the next
		// rangeController.handleRaftEvent*, which will happen at the next tick.

What does the * indicate after handleRaftEvent?

…geController

This is used to set the highest index up to which all send-queues in
pull mode must be force-flushed.

Informs cockroachdb#135601

Epic: CRDB-37515

Release note: None
Copy link
Collaborator Author

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @kvoli and @pav-kv)


pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go line 1534 at r1 (raw file):

Previously, kvoli (Austen) wrote…

We could assert on raftMu and replMu being held here as well.

Done


pkg/kv/kvserver/kvflowcontrol/rac2/range_controller.go line 2571 at r1 (raw file):

Previously, kvoli (Austen) wrote…

What does the * indicate after handleRaftEvent?

Laziness :)
Fixed.


pkg/kv/kvserver/kvflowcontrol/replica_rac2/processor.go line 612 at r1 (raw file):

Previously, kvoli (Austen) wrote…

Also here, we could add an assertion on the respective mutexes.

Done

Copy link
Contributor

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @pav-kv)

@sumeerbhola
Copy link
Collaborator Author

bors r=kvoli

@craig craig bot merged commit 7a8707d into cockroachdb:master Dec 2, 2024
kvoli added a commit to kvoli/cockroach that referenced this pull request Dec 4, 2024
Now that `Subsume` is required to force flush all replication streams up
to the lease applied index it froze the range at (cockroachdb#136321),
`SubsumeRequest` also needs to be marked as a write in order to pass
through the replication code path, which updates the range controller
force flush index and replicates the force flush index.

Part of: cockroachdb#132614
Release note: None
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.

3 participants