Skip to content

Conversation

@sumeerbhola
Copy link
Collaborator

This is the CockroachDB plumbing for Pebble's
IterOptions.OnlyReadGuaranteedDurable. It is for use in
the raftLogTruncator #76215.

Since most of the exported interfaces in the storage
package use a Reader, we support this via a
DurabilityRequirement parameter on Engine.NewReadOnly,
and not via an iterator option.

There is also a RegisterFlushCompletedCallback method
on Engine which will be used to poll certain durable
state in the raftLogTruncator.

Other than the trivial plumbing, this required some
refactoring of the Reader.MVCCGet* code for Pebble
and pebbleReadOnly. Even though it is deprecated and
primarily/only used in tests, we don't want to have
the durability semantics diverge.

Release note: None

@sumeerbhola sumeerbhola requested a review from a team February 18, 2022 21:46
@sumeerbhola sumeerbhola requested a review from a team as a code owner February 18, 2022 21:46
@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.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/storage/pebble.go, line 901 at r1 (raw file):

			atomic.AddInt64(&p.diskSlowCount, 1)
		},
		FlushEnd: func(info pebble.FlushInfo) {

Note that this does not use the Pebble code introduced in cockroachdb/pebble#1505, and instead uses EventListener as suggested by Peter cockroachdb/pebble#1505 (review)

Copy link
Collaborator

@jbowens jbowens 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 5 of 14 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jbowens)

@sumeerbhola
Copy link
Collaborator Author

TFTR!

This is the CockroachDB plumbing for Pebble's
IterOptions.OnlyReadGuaranteedDurable. It is for use in
the raftLogTruncator
cockroachdb#76215.

Since most of the exported interfaces in the storage
package use a Reader, we support this via a
DurabilityRequirement parameter on Engine.NewReadOnly,
and not via an iterator option.

There is also a RegisterFlushCompletedCallback method
on Engine which will be used to poll certain durable
state in the raftLogTruncator.

Other than the trivial plumbing, this required some
refactoring of the Reader.MVCCGet* code for Pebble
and pebbleReadOnly. Even though it is deprecated and
primarily/only used in tests, we don't want to have
the durability semantics diverge.

Release note: None
@sumeerbhola
Copy link
Collaborator Author

TestTenantLogic timeout must be a flake since nothing other than tests uses this new functionality.

@sumeerbhola
Copy link
Collaborator Author

bors r=jbowens

@craig
Copy link
Contributor

craig bot commented Feb 23, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Feb 23, 2022

Build succeeded:

@craig craig bot merged commit 5522769 into cockroachdb:master Feb 23, 2022
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