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

Add InfluxDB.tryWrite(...) methods #287

Closed
wants to merge 3 commits into from

Conversation

thomaslee
Copy link

Recently a change was introduced to optionally bound the actions queue in BatchProcessor. That's a great improvement, but the blocking semantics of queue.put(...) aren't always desirable.

We (@neeveresearch) wanted to attempt to enqueue a point in the batch. If the queue is full, we then wanted to simply drop the data and continue on with other more important things. Due to the blocking semantics, we ended up having to roll our own batching.

This PR introduces InfluxDB.tryWrite(...) methods that have similar semantics to BlockingQueue.offer(...) when batching is enabled: if the BatchProcessor's action queue is full, return false and leave it to the caller to determine what to do next. If batching is not enabled, it's equivalent to the semantics of a normal write (albeit we return true).

This change should be fully backward compatible with earlier releases at the source & bytecode level. I also modified the BatchProcessorTest to clean up threads that might otherwise leak.

@thomaslee
Copy link
Author

As an aside: I had to modify maven-compiler-plugin in the POM to build against Java 1.8 due to the use of java.util.function.Consumer in InfluxDBImpl. I changed the POM back before opening up this PR, but seems like it wouldn't compile with the settings in the POM? Am I doing something silly?

@thomaslee thomaslee changed the title Non-blocking semantics for BatchProcessor.put()/InfluxDB.write() Add InfluxDB.tryWrite(...) methods Feb 16, 2017
@majst01
Copy link
Collaborator

majst01 commented May 11, 2017

Can you please check if your requirements are solved with: #319, if yes please close this PR.

@majst01
Copy link
Collaborator

majst01 commented Sep 6, 2018

Im closing this because there was no response for a long time, @thomaslee please reopen if needed.

@majst01 majst01 closed this Sep 6, 2018
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