You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/06/18 21:25:46 UTC

[GitHub] [couchdb] noahshaw11 commented on a change in pull request #3632: Fix FDB max_document_size, binary_chunk_size, and max_attachment_size checks

noahshaw11 commented on a change in pull request #3632:
URL: https://github.com/apache/couchdb/pull/3632#discussion_r654512952



##########
File path: src/couch/src/couch_doc.erl
##########
@@ -130,7 +131,8 @@ from_json_obj_validate(EJson) ->
     from_json_obj_validate(EJson, undefined).
 
 from_json_obj_validate(EJson, DbName) ->
-    MaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),
+    ConfigMaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),

Review comment:
       I was trying to iron out a bug last night where changing the magic number to `?DOCUMENT_SIZE_LIMIT` broke the tests. I am not sure if it is flaky tests or a bug. I was getting a lot of inconsistent results last night.

##########
File path: src/couch/src/couch_att.erl
##########
@@ -680,11 +680,24 @@ is_compressible(Type) ->
 
 
 max_attachment_size() ->
-    case config:get("couchdb", "max_attachment_size", "infinity") of
+    max_attachment_size(config:get("couchdb", "max_attachment_size", "infinity")).

Review comment:
       Same here as `max_document_size`, changing the third argument in `config:get/3` breaks the tests.

##########
File path: src/couch/include/couch_db.hrl
##########
@@ -40,6 +40,8 @@
     <<"_users">>
 ]).
 
+-define(DOCUMENT_SIZE_LIMIT, 8000000).

Review comment:
       Added. See https://github.com/apache/couchdb/pull/3632/commits/64d4741bce0fe9b9fed2d5b2a3bf6816ff7a1073.

##########
File path: src/couch/src/couch_doc.erl
##########
@@ -130,7 +131,8 @@ from_json_obj_validate(EJson) ->
     from_json_obj_validate(EJson, undefined).
 
 from_json_obj_validate(EJson, DbName) ->
-    MaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),
+    ConfigMaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),

Review comment:
       No, I did not. I assume something similar should be done here. https://github.com/apache/couchdb/blob/32179742424388ef3cd0a088aa93d9c12099814f/src/couch/test/eunit/couch_doc_tests.erl#L101.

##########
File path: src/couch/src/couch_doc.erl
##########
@@ -130,7 +131,8 @@ from_json_obj_validate(EJson) ->
     from_json_obj_validate(EJson, undefined).
 
 from_json_obj_validate(EJson, DbName) ->
-    MaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),
+    ConfigMaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),

Review comment:
       No, I did not. I assume something similar should be done here. https://github.com/apache/couchdb/blob/32179742424388ef3cd0a088aa93d9c12099814f/src/couch/test/eunit/couch_doc_tests.erl#L101

##########
File path: src/couch/src/couch_doc.erl
##########
@@ -130,7 +131,8 @@ from_json_obj_validate(EJson) ->
     from_json_obj_validate(EJson, undefined).
 
 from_json_obj_validate(EJson, DbName) ->
-    MaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),
+    ConfigMaxSize = config:get_integer("couchdb", "max_document_size", 4294967296),

Review comment:
       Okay, looks like https://github.com/apache/couchdb/pull/3632/commits/fbd84fe72734a44bc61b9e7c2f9cc95548252fed fixed it.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org