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

crl-release-23.1: db: populate return statistics for flushable ingests #2429

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented Apr 3, 2023

23.1 backport of #2425.


The IngestWithStats ingestion entrypoint returnrs statistics about the ingest. Cockroach uses these statistics to inform admission control. During a flushable ingest, at commit time the number of bytes that will be ingested into L0 is unknown. This commit adds an approximation based on which tables overlapped the flushable queue. This required adjusting the memtable overlap logic to avoid short circuiting as soon as overlap is found.

This approximation is rough and may be improved with future work, such as #2112.

Informs #2421.
Informs #2112.
Informs cockroachdb/cockroach#100149.

The IngestWithStats ingestion entrypoint returnrs statistics about the ingest.
Cockroach uses these statistics to inform admission control. During a flushable
ingest, at commit time the number of bytes that will be ingested into L0 is
unknown. This commit adds an approximation based on which tables overlapped the
flushable queue. This required adjusting the memtable overlap logic to avoid
short circuiting as soon as overlap is found.

This approximation is rough and may be improved with future work, such as cockroachdb#2112.

Close cockroachdb#2421.
Informs cockroachdb#2112.
@jbowens jbowens requested review from sumeerbhola and a team April 3, 2023 16:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@bananabrick bananabrick 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: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @jbowens and @sumeerbhola)


ingest.go line 1003 at r1 (raw file):

			// just memtables.
			if metaFlushableOverlaps[i] {
				stats.ApproxIngestedIntoL0Bytes += f.Size

Could we update this during the flush? Or do we want to avoid any delay in the update of this statistic?

Copy link
Collaborator Author

@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.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @bananabrick and @sumeerbhola)


ingest.go line 1003 at r1 (raw file):

Previously, bananabrick (Arjun Nair) wrote…

Could we update this during the flush? Or do we want to avoid any delay in the update of this statistic?

This stats struct is returned directly to the caller before the flush occurs.

Copy link
Contributor

@bananabrick bananabrick left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)

@jbowens
Copy link
Collaborator Author

jbowens commented Apr 3, 2023

TFTR!

@jbowens jbowens merged commit 6de4e6c into cockroachdb:crl-release-23.1 Apr 3, 2023
@jbowens jbowens deleted the 23.1-ingest-stats branch April 3, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants