Alexey Serbin has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
Patch Set 25:
PS25, Line 187: Microseconds
> Just to be clear, I meant "coarser" where I wrote "higher" granularity. Tha
I understand the concern. What if we put 1 ms? :)
Frankly, this kind of check is not really reliable anyway. It can spot an
issue if operations are slowly pushed to tablet servers, but otherwise this
check might not catch issues.
I just could not find anything else which would not be invasive. Keeping max
value in the session itself does not seem to be a good solution. The other
solution involves building mock Batcher or Messenger which seems to be an
PS25, Line 2186: // Applying a big operation which does not fit into the buffer
: // return an error with session running in any supported flush
> Can you help me see the difference? AFAICT the second test in TestApplyTooM
Yes, after looking at TestApplyTooMuchWithoutFlushing once more I understand
you are right. For some reason I was thinking that the second part of the
TestApplyTooMuchWithoutFlushing was about big operations where each fits the
buffer, but eventually they overflow it. My bad.
OK, I'll remove that second part of the TestApplyTooMuchWithoutFlushing test.
Because this test is a superset of the former: it verifies the behavior is
correct for all flush modes.
PS25, Line 2258: WaitForAutoFlushBackground(session, FLUSH_INTERVAL_MS);
: // There should be some pending ops: the automatic
: // should not be active after switch to the MANUAL_FLUSH
> Nah, I think that's fine.
OK, I see. Thanks for the guideline.
Line 2428: // AUTO_FLUSH_BACKGROUND should be faster than AUTO_FLUSH_SYNC.
> Yes, and please try TSAN. It slows things down in unexpected ways (e.g. con
Yep, did that as well. However, I'm not sure I got 1000 cycles -- that was too
slow. Will run tonight.
PS25, Line 2574: ASSERT_TRUE(session->HasPendingOperations());
> That's fine, just loop it using dist-test.py in TSAN mode, and I'll be sati
ok, will do.
To view, visit http://gerrit.cloudera.org:8080/3952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>