Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: write ForceFlushIndex and integrate with RAC2 #136330

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sumeerbhola
Copy link
Collaborator

  • ReplicatedEvalResult.DoTimelyApplicationToAllReplicas is added and set
    in splitTriggerHelper, MergeTrigger, Migrate, Subsume.
  • The previous setting to true is gated on cluster version
    V25_1_AddRangeForceFlushKey.
  • This causes ReplicaState.ForceFlushIndex to be set, and
    RangeForceFlushKey (a replicated range-id local key) to be written when
    applying the corresponding batch to the state machine. The index is set
    to the index of the entry being applied, and is monotonically
    increasing.
  • replica_rac2.Processor and rac2.RangeController have
    existing ForceFlushIndexChangedLocked methods that are called whenever
    the Replica sees a change in the force-flush-index.

Fixes #135601

Epic: CRDB-37515

Release note: None

…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
- ReplicatedEvalResult.DoTimelyApplicationToAllReplicas is added and set
  in splitTriggerHelper, MergeTrigger, Migrate, Subsume.
- The previous setting to true is gated on cluster version
  V25_1_AddRangeForceFlushKey.
- This causes ReplicaState.ForceFlushIndex to be set, and
  RangeForceFlushKey (a replicated range-id local key) to be written when
  applying the corresponding batch to the state machine. The index is set
  to the index of the entry being applied, and is monotonically
  increasing.
- replica_rac2.Processor and rac2.RangeController have
  existing ForceFlushIndexChangedLocked methods that are called whenever
  the Replica sees a change in the force-flush-index.

Fixes cockroachdb#135601

Epic: CRDB-37515

Release note: None
Copy link

blathers-crl bot commented Nov 28, 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
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.

This is a small change. The first commit is from #136321

@kvoli There is no unit testing of whether the ForceFlushIndex is properly plumbed to the rangeController when updated, or when the Replica is initialized using a snapshot. Once your split/merge/migrate tests are merged, I think we can modify them to also introspect the state of rangeController to see that it has a ForceFlushIndex that is advancing. We could also create a new replica in one of those tests and check that the ForceFlushIndex in that replica is the same as the others. Thoughts?

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

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.

rac2: chronic send-queue in apply_to_all causes problems with splits, merges, and below-raft migrations
2 participants