nickva commented on code in PR #6013:
URL: https://github.com/apache/couchdb/pull/6013#discussion_r3617956601
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -1052,6 +1065,30 @@ header_value(Key, Headers, Default) ->
_ ->
Default
end.
+
+%% Returns true if compression is enabled for HttpDb and Body is large enough.
+compress_requests(#httpdb{request_compression = ?COMPRESS_NONE}, _BodySize) ->
+ false;
+compress_requests(#httpdb{}, BodySize) ->
Review Comment:
I wonder if we're missing a compression algorithm check here. If we get
?NONE we catch the first clause but here if we get a gzzzip and over minsize
we'd end up compressing. We should probably check explicitly, keeping a shape
of the code where we could easily add zstd or other compression methods. If we
get a bogus method we could try falling back to ?NONE but not the new gzip
default.
##########
src/couch_replicator/src/couch_replicator_api_wrap.erl:
##########
@@ -171,13 +175,15 @@ ensure_full_commit(#httpdb{} = Db) ->
get_missing_revs(#httpdb{} = Db, IdRevs) ->
JsonBody = {[{Id, couch_doc:revs_to_strs(Revs)} || {Id, Revs} <- IdRevs]},
+ RawBody = ?JSON_ENCODE(JsonBody),
+ {Body, ExtraHeaders} = maybe_compress(Db, RawBody),
Review Comment:
We have to be careful here. ?JSON_ENCODE/1 may also return iodata not just
binaries and maybe compress handles binary with a guard. The fix would be in
maybe_compress to drop the guard and use iolist_size instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]