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

allocator: change some log.VEventf to log.VInfof #109804

Closed
wants to merge 1 commit into from

Conversation

wenyihu6
Copy link
Contributor

When investigating delays in the allocator simulator test, we found that a
significant amount of time were spent on log.VEvent. Upon investigating, the log
events removed in this patch are those that occurred most frequently during
testing. I'm considering removing some of the log events here and would
appreciate a second pair of eye to better decide whether these log events are
useful for tracing.

See also: #107420
Part of: #107421
Release note: none


Before After
Screenshot 2023-08-31 at 9 14 30 AM Screenshot 2023-08-31 at 9 14 43 AM

When investigating delays in the allocator simulator test, we found that a
significant amount of time were spent on log.VEvent. Upon investigating, the log
events removed in this patch are those that occurred most frequently during
testing. I'm considering removing some of the log events here and would
appreciate a second pair of eye to better decide whether these log events are
useful for tracing.

See also: cockroachdb#107420
Part of: cockroachdb#107421
Release note: none
@blathers-crl
Copy link

blathers-crl bot commented Aug 31, 2023

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
Member

This change is Reviewable

@@ -263,7 +263,7 @@ func (rp ReplicaPlanner) PlanOneChange(
// NB: the replication layer ensures that the below operations don't cause
// unavailability; see kvserver.execChangeReplicasTxn.
action, allocatorPrio := rp.allocator.ComputeAction(ctx, rp.storePool, conf, desc)
log.KvDistribution.VEventf(ctx, 1, "next replica action: %s", action)
log.KvDistribution.VInfof(ctx, 1, "next replica action: %s", action)
Copy link
Contributor Author

@wenyihu6 wenyihu6 Aug 31, 2023

Choose a reason for hiding this comment

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

@wenyihu6 wenyihu6 closed this Oct 30, 2023
@wenyihu6 wenyihu6 deleted the log branch October 30, 2023 17:33
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.

2 participants