Skip to content

Conversation

@reltuk
Copy link
Contributor

@reltuk reltuk commented Feb 6, 2024

We make the GlobalCollector mutable.

We give the Collector a sendingThread which can have an Emitter. When the Collector collects enough events, it forwards a batch of them to the sendingThread. If the sendingThread has an Emitter, it attempts to send its current batch on that Emitter.

On shutdown, contexts get canceled, background threads get waited on, and then an accumulated un-emitted events are returned from the Collector, same as previously.

…d events while the dolt process is running.

We make the GlobalCollector mutable.

We give the Collector a sendingThread which can have an Emitter. When the
Collector collects enough events, it forwards a batch of them to the
sendingThread. If the sendingThread has an Emitter, it attempts to send its
current batch on that Emitter.

On shutdown, contexts get canceled, background threads get waited on, and then
an accumulated un-emitted events are returned from the Collector, same as
previously.
@reltuk reltuk requested a review from zachmu February 7, 2024 00:02
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
99.999562 to 99.999562
version result total
9a3ceef not ok 26
9a3ceef ok 5937431
version total_tests
9a3ceef 5937457
correctness_percentage
99.999562

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
99.999562 to 99.999562
version result total
3902282 not ok 26
3902282 ok 5937431
version total_tests
3902282 5937457
correctness_percentage
99.999562

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM!

}

const collChanBufferSize = 32
const maxBatchedEvents = 1024
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to set this so high? Seems like less bursty would be better

cancelF func()

batchCh chan []*eventsapi.ClientEvent
unsent []*eventsapi.ClientEvent
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this is actually a local var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Almost. Returned from stop() for any un-emitted things.

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
99.999562 to 99.999562
version result total
e4fe040 not ok 26
e4fe040 ok 5937431
version total_tests
e4fe040 5937457
correctness_percentage
99.999562

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
99.999562 to 99.999562
version result total
82a20f2 not ok 26
82a20f2 ok 5937431
version total_tests
82a20f2 5937457
correctness_percentage
99.999562

@reltuk reltuk merged commit 0c5dc78 into main Feb 7, 2024
@reltuk reltuk deleted the aaron/emit-metrics-while-running branch July 17, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants