[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..


Patch Set 4:

> my point was that we should make sure to stress it a bit so that
 > over time we will hit flush/flush async over different stages of
 > the auto flushing.  The way you're doing it now, you always call
 > flush at precise intervals, my suggestion was that you spin insert
 > enough rows to actually get background batches being inserted while
 > calling flush/flush async at random times, possibly increasing the
 > coverage. would that make sense?

Getting it stressed as you described certainly makes sense.  However, given 
current run times of the test I would not put that test in here.  Probably, we 
can add such a test into the integration test bundle?  Or it's OK to add a test 
which runs additional 5 minutes in TSAN configuration?

As I see it, the purpose of this test is to make sure there aren't errors, 
deadlocks or missed/unflushed batchers while calling Flush/FlushAsync in 
AUTO_FLUSH_BACKGROUND mode.  From that perspective, exploring behavior of the 
client/server in all phases besides the very first one it is no different than 
calling Flush/AsyncFlush in any other regular session.  The only phase of 
interest in that context is when there is contention on detaching the current 
batcher by auto-flusher and the call of explicit flush.

What do you think?

-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..


Patch Set 4:

my point was that we should make sure to stress it a bit so that over time we 
will hit flush/flush async over different stages of the auto flushing.  The way 
you're doing it now, you always call flush at precise intervals, my suggestion 
was that you spin insert enough rows to actually get background batches being 
inserted while calling flush/flush async at random times, possibly increasing 
the coverage. would that make sense?

-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4492

to look at the new patch set (#4).

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..

[client-test] one more test for AUTO_FLUSH_BACKGROUND

An additional test for the AUTO_FLUSH_BACKGROUND flush mode:
verify that it's safe to perform synchronous and/or asynchronous
flush while having the auto-flusher thread running in the background.

Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
---
M src/kudu/client/client-test.cc
1 file changed, 22 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4492/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4492/3/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

Line 2435:   const size_t kIterNum = AllowSlowTests() ? 1024 : 256;
> I apologize for not saying the first time around.
I should have asked for the driver for your request :)

Sure.  The essence of this test is to have explicit and background flushes run 
in parallel, having some contention sometimes and making sure no errors 
appears.  There is no need to insert a lot of rows -- in that sense there is no 
difference having huge or small buffers to flush.

Yes, I'll add a check to verify that rows have landed in the DB.


-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4492

to look at the new patch set (#2).

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..

[client-test] one more test for AUTO_FLUSH_BACKGROUND

An additional test for the AUTO_FLUSH_BACKGROUND flush mode:
verify that it's safe to perform synchronous and/or asynchronous
flush while having the auto-flusher thread running in the background.

Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
---
M src/kudu/client/client-test.cc
1 file changed, 20 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4492/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4492/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

Line 2439:   for (size_t i = 0; i < 256; i += 2) {
this seems too short in terms ops and time, can you have this run for longer in 
slow tests?


-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [client-test] one more test for AUTO FLUSH BACKGROUND

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4492

Change subject: [client-test] one more test for AUTO_FLUSH_BACKGROUND
..

[client-test] one more test for AUTO_FLUSH_BACKGROUND

An additional test for the AUTO_FLUSH_BACKGROUND flush mode:
verify that it's safe to perform synchronous and/or asynchronous
flush while having the auto-flusher thread running in the background.

Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
---
M src/kudu/client/client-test.cc
1 file changed, 19 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/4492/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3bd5d248d4d44393689c8da81ed669395c393257
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin