chore: optimize single-record blocking write #297
Conversation
pabigot
force-pushed
the
pabigot/20220103a
branch
from
13c375d to
d12451f
Compare
Codecov Report
@@ Coverage Diff @@
## master #297 +/- ##
==========================================
+ Coverage 91.87% 91.89% +0.01%
==========================================
Files 23 23
Lines 2042 2047 +5
==========================================
+ Hits 1876 1881 +5
Misses 110 110
Partials 56 56
Continue to review full report at Codecov.
|
| if len(line) == 0 { | ||
| return nil | ||
| } | ||
| if len(line) == 1 { |
There was a problem hiding this comment.
empty lines are ignored by InfluxDB, adding extra new line is not required, it could simply return w.write(ctx, line[0])
There was a problem hiding this comment.
The existing implementation assumes there are no terminating newlines on the passed records; it's necessary to add one when missing to keep the existing behavior. Also, not adding a newline causes the tests to fail, and keeps this consistent with what's produced by line-protocol, see influxdata/line-protocol#52.
There was a problem hiding this comment.
BTW: That existing behavior is verified by the tests, which is why it isn't the way you suggested....
| } | ||
| return nil | ||
| var sb strings.Builder |
There was a problem hiding this comment.
a nit: this could be also optimized to avoid useless allocations of temporary buffers, it could Grow in advance to len(line)+sum_of_line_lengths
There was a problem hiding this comment.
strings.Join(lines, "\n") would probably be best, except for the need to add a terminal newline. Lifting []bytes out of the existing loop and expanding it at need based on line length could also work. I didn't want to make that large a change, but if it's important enough I'll take a stab at it.
There was a problem hiding this comment.
yes strings.Join is already an optimized code and it would be the best option, though it would probably break the tests. The original goal herein (doc + simple optimization for 1 line) is already the necessary improvement.
The level and depth of optimizations is up to you ... it was just a nit from me. @vlastahajek is the approver herein.
pabigot
force-pushed
the
pabigot/20220103a
branch
from
d12451f to
0cf22fe
Compare
|
rebase to confirm no conflicts |
Proposed Changes
This optimizes
WriteAPIBlocking.WriteRecordin the case where a single string is provided, and explicitly documents that batches (records separated/terminated by newline) can be passed. A test is added to confirm that this works as expected.Unless the server chokes on empty lines this should resolve the need that motivated #295.
Checklist