Skip to content

kvadmission: remove grunning non-monotonic comment#126424

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
aadityasondhi:20240628.remove-mononocity-comment
Jul 2, 2024
Merged

kvadmission: remove grunning non-monotonic comment#126424
craig[bot] merged 1 commit intocockroachdb:masterfrom
aadityasondhi:20240628.remove-mononocity-comment

Conversation

@aadityasondhi
Copy link
Copy Markdown
Contributor

This was fixed in #118907.

This patch adds an assertion for test builds to ensure monotonic grunning time.

Informs #95529

Release note: None

This was fixed in cockroachdb#118907.

This patch adds an assertion for test builds to ensure monotonic
grunning time.

Informs cockroachdb#95529

Release note: None
@aadityasondhi aadityasondhi requested a review from a team as a code owner June 28, 2024 19:59
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Jun 28, 2024

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @aadityasondhi)


pkg/kv/kvserver/kvadmission/kvadmission.go line 455 at r1 (raw file):

	if ah.callAdmittedWorkDoneOnKVAdmissionQ {
		cpuTime := grunning.Time() - ah.cpuStart
		if buildutil.CrdbTestBuild && cpuTime < 0 {

if someone uses an unpatched runtime, will cpuTime be 0?

Copy link
Copy Markdown
Contributor Author

@aadityasondhi aadityasondhi left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r=sumeerbhola

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)


pkg/kv/kvserver/kvadmission/kvadmission.go line 455 at r1 (raw file):

Previously, sumeerbhola wrote…

if someone uses an unpatched runtime, will cpuTime be 0?

Yes. Both grunning.Time() and ah.cpuStart will be 0 in unpatched versions.

ah.cpuStart = grunning.Time()

@aadityasondhi
Copy link
Copy Markdown
Contributor Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 2, 2024

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