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

Batch is not thread safe #47

Open
hsanjuan opened this issue Oct 25, 2018 · 2 comments
Open

Batch is not thread safe #47

hsanjuan opened this issue Oct 25, 2018 · 2 comments
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature

Comments

@hsanjuan
Copy link
Contributor

Looking at batch code, it using an activeCommits int and size int counters very liberally all over the place. Calling Add(), at the same time as Commit() seems likely to trigger weird side effects: i.e. size reset to 0 when it should not, or activeCommits potentially going on the negative.

@Stebalien
Copy link
Member

Batch was never intended to be thread-safe. We should probably document that.

We could also just make it thread-safe. Given that we have to spin off goroutines anyways, that may not be a bad idea.

For context, batch used to be extremely simple and flushed synchronously. We had to change that to prevent periodic pauses on add.

@hsanjuan
Copy link
Contributor Author

I'll add a note for the moment

@hsanjuan hsanjuan added help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature labels Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants