nickva commented on code in PR #6114:
URL: https://github.com/apache/couchdb/pull/6114#discussion_r4005789950


##########
src/couch/test/eunit/couch_file_tests.erl:
##########
@@ -21,6 +21,8 @@
 setup() ->
     {ok, Fd} = couch_file:open(?tempfile(), [create, overwrite]),
     meck:new(file, [passthrough, unstick]),
+    config:set_boolean("couch_file", "append_bin_eof_check", true),

Review Comment:
   In the setup we usually set the value with a `Persist=false` option. 
Otherwise this pollutes the tests default.ini file for all subsequent tests.



##########
src/couch/src/couch_file.erl:
##########
@@ -1017,6 +1028,22 @@ dup(#file{fd = Fd} = File) ->
             File
     end.
 
+validate_eof(#file{} = File, Key, Default) when is_list(Key), 
is_boolean(Default) ->
+    case config:get_boolean("couch_file", Key, Default) of
+        false ->
+            ok;
+        true ->
+            {ok, ActualPos} = file:position(File#file.fd, eof),
+            if
+                File#file.eof /= ActualPos ->
+                    throw_stop(
+                        {eof_mismatch, Key, ActualPos, File#file.eof, 
File#file.filepath}, File

Review Comment:
   We have a metric for `couch_stats:increment_counter([pread, exceed_eof])` 
wonder if header eof mismatches are also important enough to be a metric...



##########
src/couch/src/couch_file.erl:
##########
@@ -623,9 +624,9 @@ handle_info({'DOWN', Ref, process, _Pid, _Info}, 
#file{db_monitor = Ref} = File)
 eof(#file{fd = Fd}) ->
     file:position(Fd, eof).
 
-append_bins(#file{fd = Fd} = File, Bins) ->
-    {ok, Pos} = eof(File),
-    {BlockResps, _} = lists:mapfoldl(
+append_bins(#file{fd = Fd, eof = Pos} = File, Bins) ->
+    validate_eof(File, "append_bin_eof_check", false),

Review Comment:
   Should we skip validation for appends?  If we validate in the header we 
won't be able to commit, crash and end up looking  for the previous header 
anyway. We'd just generate more junk in the meantime. But these happens pretty 
rarely.



##########
src/couch/src/couch_file.erl:
##########
@@ -1017,6 +1028,22 @@ dup(#file{fd = Fd} = File) ->
             File
     end.
 
+validate_eof(#file{} = File, Key, Default) when is_list(Key), 
is_boolean(Default) ->
+    case config:get_boolean("couch_file", Key, Default) of

Review Comment:
   Add a line and a short comment in default.ini for 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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to