[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 AlvesTested-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 Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [tests] MANUAL FLUSH --> AUTO FLUSH BACKGROUND
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 SerbinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[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