implement query() call to make asynchronous InfluxDb query with TimeUnit #418
Conversation
Codecov Report
@@ Coverage Diff @@
## master #418 +/- ##
===========================================
- Coverage 84.57% 84.38% -0.2%
- Complexity 232 236 +4
===========================================
Files 17 17
Lines 1018 1031 +13
Branches 96 96
===========================================
+ Hits 861 870 +9
- Misses 108 111 +3
- Partials 49 50 +1
Continue to review full report at Codecov.
|
|
Can you please explain a bit more why this is needed. At least a unit test showing it works is required. |
|
sure, happy to. about the unit test - do I create a new pull request with the unit test included or can I add it to this one somehow? sorry for asking a stupid question, I do not do pull requests very often. about the justification: it comes down to the ability to satisfy two requirements: 1) be able to pass our own preconfigured http client for SSL auth and rate limiting and 2) to get timestamps in microseconds instead of as strings in RFC3339 format your library uses Retrofit2 and so do we. Our application uses Inflixdb as a generic time series database, along with 3 more different TSDB implementations that we support (e.g. Graphite). It is possible for a user to run several instances of our application on-premise and all these instances can share the same Influxdb server. We need to be able to rate-limit queries these instances can send to Influxdb at the same time. Without this rate limiting, it is quite possible for one instance of our app to fire few thousand queries at the same time. Retrofit2 (or rather okhttp3 they use) provides a way to control the parallelism of the queries they make through the configuration of the Dispatcher. You have a variant of the query() call that accepts Builder which is a way to pass http client that we create and configure to the inflixdb library. We configure the Dispatcher used by our client to limit the number of simultaneous connections to the same host (just what we need). One has to call retrofit2 enqueue() to utilize the rate limiting. Their call execute() is blocking and executes in the caller's thread so it does not use the Dispatcher. You can configure Dispatcher with a reasonable maxRequestsPerHost and still fire thousands of queries in parallel if you call execute() using your own thread pool. Another important reason to use the variant of the query() call where we pass our own HTTP client is to be able to preconfigure HTTP client with SSL truststore and keystore for SSL auth when Influxdb runs with SSL About the timestamps: we use Influxdb as a time series storage and the data we get from the query() call can be used to do various things before it is presented to the user. For example, we need timestamps in microseconds when we perform calculations with the data, or when we build graphs. Issue #339 explains difficulties with converting the time from RFC3339 to timestamps, besides, InfluxDB API has a way to request the time in microseconds anyway (or milliseconds or any other units). You have a variant of the query() call that accepts the parameter for that, but this call is blocking because it uses retrofit2 execute() instead of enqueue(). These are the reasons why query() call that can accept TimeUnit parameter and uses enqueue() to run actual InfluxDb API query is useful. |
…call with and without TimeInit parameter
|
Hi @vkurland if still interested please rebase and resolve the merge conflicts. |
3 participants
Hi
This change introduces a non-blocking query() call that also allows for the TimeUnit parameter to get timestamps in microseconds or nanoseconds rather than in RFC3339. Currently (in the upstream master) we either can do a non-blocking call (with onSuccess and onFailure callbacks) but get timestamps in RFC3339, or specify timestamp unit but then the call is blocking. This change provides a way to have both.
thank you
Vadim Kurland