You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/09/20 01:41:16 UTC
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4471
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 50 insertions(+), 75 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/1
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 878
> Yep, it's passing. I thought it would be enough to call inserted_idx_.Stor
Oh, that's clear -- what happens is that all the inserts are reported in the very end of the AlterTableTest::InserterThread() routine/method. So, this test boiled down to making a single update in the very end, and it does not update a single row (since 'inserted_idx' points to the next index which would be inserted if the test continued to run).
OK, what I will do is the following:
1. Return back periodic flushes every 50 rows (it works with the AUTO_FLUSH_BACKGROUND mode as well)
2. Check that at least one update operations is applied to the session.
Let me know if you think some more sanity checks are required.
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4471
to look at the new patch set (#2).
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 49 insertions(+), 82 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/2
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4471/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:
Line 218: // Set the flush threshold low so that there are flush events which
> grammar changes like this are subjective, we usually avoid doing them if yo
ok, that makes sense -- will return it back then.
Line 222: // Set the major delta compaction ratio low enough so that the test
> I'm ambivalent whether the grammar change is justified.
Done
Line 227: // once KUDU-1631/KUDU-1346 is fixed. It's necessary to change the default
> you mean KUDU-1632, not KUDU-1631. Maybe mark KUDU-1632 as a dup of KUDU-13
Done
PS3, Line 231: --consensus_max_batch_size_bytes=2097152
> have you tested that this doens't trigger the check. How big can single tab
Yes, I've tested that. As you could see from the stack trace of the debug assert, it was slightly bigger than 1MiB:
request->ByteSize() <= FLAGS_consensus_max_batch_size_bytes (1168286 vs. 1048576)
*** Check failure stack trace: ***
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4471
to look at the new patch set (#3).
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 92 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/3
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Reviewed-on: http://gerrit.cloudera.org:8080/4471
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 91 insertions(+), 101 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4471/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:
PS3, Line 231: --consensus_max_batch_size_bytes=2097152
> yeah, in that case, I'm just afraid this will get flaky, but I guess we can
Well, it did not expose flakiness in dist_test so far. But let's see.
http://gerrit.cloudera.org:8080/#/c/4471/5/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 264:
> if this is the next row now can you change the variable name?
Good idea! Will do.
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4471
to look at the new patch set (#4).
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 92 insertions(+), 100 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/4
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4471
to look at the new patch set (#5).
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 90 insertions(+), 100 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/5
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 878
again, you removed the update of the atomic variable. How is this test still passing?
Line 914
> Good catch -- yes, it's a mistake. Will fix.
What I'm trying to figure out is why the test didn't fail on jenkins. Are we missing some assertion somewhere?
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 878
> again, you removed the update of the atomic variable. How is this test stil
Yep, it's passing. I thought it would be enough to call inserted_idx_.Store() after flushing the operations. I will clarify on this.
Line 914
> What I'm trying to figure out is why the test didn't fail on jenkins. Are w
Yes, you are right -- even without increment of the 'i' counter, the tests pass (at least in release, ASAN and TSAN configurations).
From what I can see, passing the tests with missing increment for the counter is not surprising -- there isn't unique constraint on the second columns (column idx 1), so there should not be an issue if the counter stays the same. Basically, it means that counter is not crucial here.
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4471
to look at the new patch set (#6).
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
[tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
In tests, run KuduSession in AUTO_FLUSH_BACKGROUND instead of
MANUAL_FLUSH mode where appropriate.
Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
---
M src/kudu/client/client-test.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/flex_partitioning-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tools/ksck_remote-test.cc
11 files changed, 91 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/4471/6
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 5:
(2 comments)
oops had a unpublished comment. my apologies.
http://gerrit.cloudera.org:8080/#/c/4471/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:
PS3, Line 231:
> Yes, I've tested that. As you could see from the stack trace of the debug
yeah, in that case, I'm just afraid this will get flaky, but I guess we can just push and see.
http://gerrit.cloudera.org:8080/#/c/4471/5/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 264: atomic<uint32_t> inserted_idx_;
if this is the next row now can you change the variable name?
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4471/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:
Line 218: // Set the flush threshold low so that there are flush events which
grammar changes like this are subjective, we usually avoid doing them if you're not actually touching that part of the code. Fixing typos and grammar errors is ok, but rephrasing stuff differently should be avoided as it's polluting the history. I for one prefer the previous version, since I don't recall we calling flushes "flush events" anywhere else.
Line 222: // Set the major delta compaction ratio low enough so that the test
I'm ambivalent whether the grammar change is justified.
Line 227: // once KUDU-1631/KUDU-1346 is fixed. It's necessary to change the default
you mean KUDU-1632, not KUDU-1631. Maybe mark KUDU-1632 as a dup of KUDU-1346 and close the former?
PS3, Line 231: --consensus_max_batch_size_bytes=2097152
have you tested that this doens't trigger the check. How big can single tablet batches get?
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS1, Line 3502: {
sprurious change
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 914
hum, you removed the statement that incremented 'i' this test still passes?
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [tests] MANUAL_FLUSH --> AUTO_FLUSH_BACKGROUND
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS1, Line 3502: {
> sprurious change
ok, will return back.
http://gerrit.cloudera.org:8080/#/c/4471/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:
Line 914
> hum, you removed the statement that incremented 'i' this test still passes?
Good catch -- yes, it's a mistake. Will fix.
--
To view, visit http://gerrit.cloudera.org:8080/4471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieafc198609cceb5d6945a910364056d81786629a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes