[ISSUE-520] Pool gzip compressor and buffer used in gzip writer #521
Conversation
pakio
requested review from
dblock,
VijayanB,
VachaShah and
Jakob3xD
as code owners
pakio
force-pushed
the
ISSUE-520_pool-gzip-compressor
branch
2 times, most recently
from
5629a3c to
66b4bf9
Compare
|
I will update changelog once the change is finalized. |
opensearchtransport/gzip.go
Outdated
| // Modifications Copyright OpenSearch Contributors. See | ||
| // GitHub history for details. | ||
|
|
||
| // Licensed to Elasticsearch B.V. under one or more contributor |
There was a problem hiding this comment.
Is this code copied from open source Elasticsearch client or is this new? If it's new, you don't need to reproduce the "Licensed to ES" part of the license.
opensearchtransport/gzip_test.go
Outdated
| t.Run("gzip multiple times", func(t *testing.T) { | ||
| gzipCompressor := newGzipCompressor() | ||
| body := `{"query":{"match_all":{}}}` | ||
| for i := 0; i < 5; i++ { |
There was a problem hiding this comment.
Maybe generate a different/random/based on i body every time? Feels like a missed opportunity :)
opensearchtransport/gzip_test.go
Outdated
| t.Fatal("expected body to be the same after compressing and decompressing") | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
If the compressor is a noop and just returns the same string, these tests pass.
- Add a test that compressed data is different and smaller than the original data.
- Add a test that decompressing a random buffer returns the expected errors.
- Add a test that compresses data, then compresses it again, and decompresses it twice to get to source, ensuring nothing is lost.
Rebase, we may have fixed this elsewhere since the |
pakio
force-pushed
the
ISSUE-520_pool-gzip-compressor
branch
from
66b4bf9 to
fe6bd68
Compare
Signed-off-by: Kazuma (Pakio) Arimura <k.arimura96@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #521 +/- ##
==========================================
+ Coverage 57.29% 66.21% +8.92%
==========================================
Files 315 364 +49
Lines 9823 8587 -1236
==========================================
+ Hits 5628 5686 +58
+ Misses 2902 1589 -1313
- Partials 1293 1312 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pakio
force-pushed
the
ISSUE-520_pool-gzip-compressor
branch
2 times, most recently
from
d40fcd7 to
a0851ed
Compare
Signed-off-by: Kazuma (Pakio) Arimura <k.arimura96@gmail.com>
Signed-off-by: Kazuma (Pakio) Arimura <k.arimura96@gmail.com>
pakio
force-pushed
the
ISSUE-520_pool-gzip-compressor
branch
from
a0851ed to
bc5b63d
Compare
Signed-off-by: Kazuma (Pakio) Arimura <k.arimura96@gmail.com>
Signed-off-by: Kazuma (Pakio) Arimura <k.arimura96@gmail.com>
|
oh I was about to send some message regarding the change. thanks you @dblock for your review and merging the PR! |
|
Good work @pakio, thank you! |
Description
Changed to pool
gzip.Writerobject andbytes.Bufferobject used for gzip compression, for the better performance.Issues Resolved
Closes #520
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.