Skip to content

Conversation

@mcpherrinm
Copy link
Contributor

@mcpherrinm mcpherrinm commented Oct 30, 2025

Instead of spawning all goroutines immediately with their own sleeps, this uses a ticker on the stagger interval to start additional submissions.

This shouldn't change much, but unblocks future improvements like kicking off additional submissions immediately on failure to submit to a log, or adding more logic to what order we submit in based on the current results.

As a small perf win, this should slightly decrease the number of goroutines as we don't typically have to submit to too many logs.

This is a small refactor to remove a pair of closures so we can directly launch
the submission goroutines with `go ctp.getOne(...)`.

This is itself pulled out of a slightly larger refactor which pulls the stagger
sleep back out of this function, but it's much cleaner to do this refactor as
a preliminary change.

There is a small functional change here: The same subCtx is passed to the
submission RPC. I think that shouldn't cause any problems.
Instead of spawning all goroutines immediately with their own sleeps, this uses
a ticker on the stagger interval to start additional submissions.

This shouldn't change much, but unblocks future improvements like kicking off
additional submissions immediately on failure to submit to a log, or adding
more logic to what order we submit in based on the current results.

As a small perf win, this should slightly decrease the number of goroutines as
we don't typically have to submit to too many logs.
@mcpherrinm mcpherrinm changed the base branch from main to mattm-refactor-get-one-sct October 30, 2025 22:00
@mcpherrinm mcpherrinm marked this pull request as ready for review October 30, 2025 22:31
@mcpherrinm mcpherrinm requested a review from a team as a code owner October 30, 2025 22:31
@mcpherrinm mcpherrinm requested review from beautifulentropy and removed request for a team October 30, 2025 22:31
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Heck yes, I'm really happy you found a way to make this ticker-based instead of kicking off all the goroutines at once. This is going to make future changes so much easier.

keep it closer to initialization
len(logs) no longer changes
@mcpherrinm mcpherrinm changed the base branch from mattm-refactor-get-one-sct to main November 1, 2025 00:59
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

LGTM mod doccomment.

mcpherrinm and others added 4 commits November 4, 2025 09:33
This was needed when an earlier version of this code was mutating logs.

Since it isn't needed anymore, we can just inline it.
This makes a bit more sense where the check is
@mcpherrinm mcpherrinm requested a review from aarongable November 4, 2025 16:07
@aarongable aarongable merged commit 8a445fa into main Nov 6, 2025
20 of 22 checks passed
@aarongable aarongable deleted the mattm-refactor-ticker branch November 6, 2025 16:42
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.

4 participants