[kudu-CR] generic iterators: assorted cleanup
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12156 to look at the new patch set (#3). Change subject: generic_iterators: assorted cleanup .. generic_iterators: assorted cleanup This patch cleans up the MergeIterator and makes it look more like the UnionIterator: 1. Addressed a long-standing TODO about schema verification. 2. Extracted the schema from the sub-iterators. Change-Id: I7f1fd5f4cd1a8ef621d3ac994e764dfd19e99544 --- M src/kudu/common/generic_iterators-test.cc M src/kudu/common/generic_iterators.cc M src/kudu/common/generic_iterators.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/tablet.cc 5 files changed, 95 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/12156/3 -- To view, visit http://gerrit.cloudera.org:8080/12156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f1fd5f4cd1a8ef621d3ac994e764dfd19e99544 Gerrit-Change-Number: 12156 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] generic iterators: assorted cleanup
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12156 to look at the new patch set (#2). Change subject: generic_iterators: assorted cleanup .. generic_iterators: assorted cleanup This patch cleans up the MergeIterator and makes it look more like the UnionIterator: 1. Addressed a long-standing TODO about schema verification. 2. Extracted the schema from the sub-iterators. Change-Id: I7f1fd5f4cd1a8ef621d3ac994e764dfd19e99544 --- M src/kudu/common/generic_iterators-test.cc M src/kudu/common/generic_iterators.cc M src/kudu/common/generic_iterators.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/tablet.cc 5 files changed, 93 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/12156/2 -- To view, visit http://gerrit.cloudera.org:8080/12156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f1fd5f4cd1a8ef621d3ac994e764dfd19e99544 Gerrit-Change-Number: 12156 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] generic iterators: short-circuit MergeIterState::PullNextBlock
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12157 to look at the new patch set (#2). Change subject: generic_iterators: short-circuit MergeIterState::PullNextBlock .. generic_iterators: short-circuit MergeIterState::PullNextBlock This is a micro-optimization to avoid testing every bit in the selection vector when we've already counted up the number of set bits. Change-Id: I529e632f40943962d34c184b29b5cff7604ad59b --- M src/kudu/common/generic_iterators.cc 1 file changed, 6 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/12157/2 -- To view, visit http://gerrit.cloudera.org:8080/12157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I529e632f40943962d34c184b29b5cff7604ad59b Gerrit-Change-Number: 12157 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] tablet: clean up MergeIterState API
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12176 ) Change subject: tablet: clean up MergeIterState API .. tablet: clean up MergeIterState API This patch simply adds documentation and some readability cleanup to MergeIterState, which is a helper class for the MergeIterator. Non-API changes: - Add API docs to all public methods. - Extract the implementation of Advance() and PullNextBlock() out of the class declaration to make the API contract easier to quickly read and understand. - Rename a couple of private variables to clarify row-oriented semantics. API cleanup: - Add a public Init() method that delegates to PullNextBlock(). - Make PullNextBlock() and IsBlockExhausted() private methods. - Replace exposure of the underlying iterator via iter() with a public AddStats() method, since stats collection was the only use of iter(). There are no functional changes in this patch. Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Reviewed-on: http://gerrit.cloudera.org:8080/12176 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/common/generic_iterators.cc 1 file changed, 89 insertions(+), 64 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Gerrit-Change-Number: 12176 Gerrit-PatchSet: 5 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] Support location awareness in READ CLOSEST for the Java client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12175 ) Change subject: Support location awareness in READ_CLOSEST for the Java client .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12175/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: http://gerrit.cloudera.org:8080/#/c/12175/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@111 PS3, Line 111: !location.isEmpty() && nit: isn't this part redundant once the other two are present? -- To view, visit http://gerrit.cloudera.org:8080/12175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief0f07058cefd0037f4b0f7c60c8b7809dc8313f Gerrit-Change-Number: 12175 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 09 Jan 2019 07:04:16 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2195. Add additional gflag to force sync of consensus metadata
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12186 ) Change subject: KUDU-2195. Add additional gflag to force sync of consensus metadata .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73212d1670a33479cce7d9ef9ee61cfe9b00cdd3 Gerrit-Change-Number: 12186 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 09 Jan 2019 06:48:11 + Gerrit-HasComments: No
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 ) Change subject: Support location awareness in READ_CLOSEST for the C++ client .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 8 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 09 Jan 2019 06:52:10 + Gerrit-HasComments: No
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12178 ) Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 09 Jan 2019 06:49:18 + Gerrit-HasComments: No
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12122 ) Change subject: KUDU-2543 pt 2: pass around default authz tokens .. Patch Set 5: (12 comments) http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/authz_token_cache.h File src/kudu/client/authz_token_cache.h: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/authz_token_cache.h@134 PS5, Line 134: retrieve_authz_rpcs_ nit: why not name it 'authz_rpcs_' to match 'authz_tokens_'? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-internal.cc@466 PS5, Line 466: authz_token_cache_.Put nit: use StoreAuthzToken() instead? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-test.cc@5853 PS5, Line 5853: client_->data_->RetrieveAuthzTokenAsync(client_table_.get(), s.AsStatusCallback(), Is it possible to change the logic to create a new client and only retrieve authz token when there isn't one cached? So that we can have a deterministic result that there is only one RPC being sent in this case? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc File src/kudu/integration-tests/auth_token_expire-itest.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc@143 PS5, Line 143: nit: extra space. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc@254 PS5, Line 254: nit: space. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc File src/kudu/integration-tests/authz_token-itest.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@184 PS5, Line 184: Insert nit: 'Inserts' to aligned with other comments style. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@205 PS5, Line 205: Get nit: 'Gets' http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@281 PS5, Line 281: LOG(INFO) << "Trying to use the wrong user's token..."; Can you add a comment to explain what is the expected behavior in this case? Especially why operations in L285 is expected to succeed. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@362 PS5, Line 362: ASSERT_OK(ScanFromTable(client_table_.get())); Why not parameterized the functor here as well? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@369 PS5, Line 369: TestUnknownTsk Should a similar test be exist for authn token as well? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@477 PS5, Line 477: TestMasterElectionStorms Will a similar also apply for authn tokens? Also, wondering how you consider what should be tested in auth_token_expire-itest and what should be in authz_token-itest? http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@523 PS5, Line 523: // Ensures the client can still communicate with servers that do not support : // authz tokens. What happens if a old client try to communicate with servers that require authz tokens? -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Gerrit-Change-Number: 12122 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 09 Jan 2019 06:31:27 + Gerrit-HasComments: Yes
[kudu-CR] [fs]: wrapping up containers in scoped refptr
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12121 ) Change subject: [fs]: wrapping up containers in scoped_refptr .. Patch Set 6: > Here is a tserver node of our online cluster, and it has been running for > months. So, it has more than 38k+ containers and almost 20k+ are dead. Looks like it's got many more containers than that: > Total number of LBM containers: 206682 (147 full) In any case, thank you for the additional testing. The results are very nice. Looks like the slowdown is 1-2%, which is well within the possible variance for startup time anyway. I'll merge this. -- To view, visit http://gerrit.cloudera.org:8080/12121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 Gerrit-Change-Number: 12121 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 09 Jan 2019 06:21:17 + Gerrit-HasComments: No
[kudu-CR] [fs]: wrapping up containers in scoped refptr
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12121 ) Change subject: [fs]: wrapping up containers in scoped_refptr .. [fs]: wrapping up containers in scoped_refptr It's necessary to wrap up containers in scoped_refptr to support deleting the full containers that are dead at runtime. Based on this, the KUDU-2636 can be fixed. Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 Reviewed-on: http://gerrit.cloudera.org:8080/12121 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h 3 files changed, 129 insertions(+), 141 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 Gerrit-Change-Number: 12121 Gerrit-PatchSet: 7 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu
[kudu-CR] [fs]: wrapping up containers in scoped refptr
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12121 ) Change subject: [fs]: wrapping up containers in scoped_refptr .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 Gerrit-Change-Number: 12121 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 09 Jan 2019 06:21:35 + Gerrit-HasComments: No
[kudu-CR] [fs]: wrapping up containers in scoped refptr
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12121 ) Change subject: [fs]: wrapping up containers in scoped_refptr .. Patch Set 6: Here is a tserver node of our online cluster, and it has been running for months. So, it has more than 38k+ containers and almost 20k+ are dead. Test step: 1) rebooted the node to repair the containers; 2) rebooted the node with none scoped_refptr version; 3) rebooted the node with scoped_refptr version; Step 2's result: I0109 13:37:40.599534 159639 fs_manager.cc:419] Time spent opening block manager: real 58.437s user 0.000s sys 0.000s I0109 13:37:40.599802 159639 fs_manager.cc:436] Opened local filesystem: /mnt/dfs/0/kudu/tserver/data,/mnt/dfs/1/kudu/tserver/data,/mnt/dfs/2/kudu/tserver/data,/mnt/dfs/3/kudu/tserver/data,/mnt/dfs/4/kudu/tserver/data,/mnt/dfs/5/kudu/tserver/data,/mnt/dfs/6/kudu/tserver/data,/mnt/dfs/7/kudu/tserver/data,/mnt/dfs/8/kudu/tserver/data,/mnt/dfs/9/kudu/tserver/data,/mnt/dfs/10/kudu/tserver/data,/mnt/dfs/ssd0/kudu/tserver/wal uuid: "89c5dd62ad734d54b7d25bc2d52263d3" format_stamp: "Formatted at 2018-03-12 07:20:41 on kudu14.lt.163.org" I0109 13:37:40.599843 159639 fs_report.cc:352] FS layout report wal directory: /mnt/dfs/ssd0/kudu/tserver/wal metadata directory: /mnt/dfs/0/kudu/tserver/data 11 data directories: /mnt/dfs/0/kudu/tserver/data/data, /mnt/dfs/1/kudu/tserver/data/data, /mnt/dfs/2/kudu/tserver/data/data, /mnt/dfs/3/kudu/tserver/data/data, /mnt/dfs/4/kudu/tserver/data/data, /mnt/dfs/5/kudu/tserver/data/data, /mnt/dfs/6/kudu/tserver/data/data, /mnt/dfs/7/kudu/tserver/data/data, /mnt/dfs/8/kudu/tserver/data/data, /mnt/dfs/9/kudu/tserver/data/data, /mnt/dfs/10/kudu/tserver/data/data Total live blocks: 23590554 Total live bytes: 1449771979998 Total live bytes (after alignment): 1529197359104 Total number of LBM containers: 206682 (147 full) Did not check for missing blocks Did not check for orphaned blocks Total full LBM containers with extra space: 0 (0 repaired) Total full LBM container extra space in bytes: 0 (0 repaired) Total incomplete LBM containers: 0 (0 repaired) Total LBM partial records: 0 (0 repaired) I0109 13:37:40.62 159639 env_posix.cc:1676] Not raising this process' running threads per effective uid limit of 1033414; it is already as high as it can go I0109 13:37:40.633038 159639 ts_tablet_manager.cc:344] Loading tablet metadata (0/1989 complete) Step 3's result: I0109 13:43:06.445422 160217 fs_manager.cc:419] Time spent opening block manager: real 59.316s user 0.000s sys 0.004s I0109 13:43:06.445688 160217 fs_manager.cc:436] Opened local filesystem: /mnt/dfs/0/kudu/tserver/data,/mnt/dfs/1/kudu/tserver/data,/mnt/dfs/2/kudu/tserver/data,/mnt/dfs/3/kudu/tserver/data,/mnt/dfs/4/kudu/tserver/data,/mnt/dfs/5/kudu/tserver/data,/mnt/dfs/6/kudu/tserver/data,/mnt/dfs/7/kudu/tserver/data,/mnt/dfs/8/kudu/tserver/data,/mnt/dfs/9/kudu/tserver/data,/mnt/dfs/10/kudu/tserver/data,/mnt/dfs/ssd0/kudu/tserver/wal uuid: "89c5dd62ad734d54b7d25bc2d52263d3" format_stamp: "Formatted at 2018-03-12 07:20:41 on kudu14.lt.163.org" I0109 13:43:06.445729 160217 fs_report.cc:352] FS layout report wal directory: /mnt/dfs/ssd0/kudu/tserver/wal metadata directory: /mnt/dfs/0/kudu/tserver/data 11 data directories: /mnt/dfs/0/kudu/tserver/data/data, /mnt/dfs/1/kudu/tserver/data/data, /mnt/dfs/2/kudu/tserver/data/data, /mnt/dfs/3/kudu/tserver/data/data, /mnt/dfs/4/kudu/tserver/data/data, /mnt/dfs/5/kudu/tserver/data/data, /mnt/dfs/6/kudu/tserver/data/data, /mnt/dfs/7/kudu/tserver/data/data, /mnt/dfs/8/kudu/tserver/data/data, /mnt/dfs/9/kudu/tserver/data/data, /mnt/dfs/10/kudu/tserver/data/data Total live blocks: 23590554 Total live bytes: 1449771979998 Total live bytes (after alignment): 1529197359104 Total number of LBM containers: 206682 (147 full) Did not check for missing blocks Did not check for orphaned blocks Total full LBM containers with extra space: 0 (0 repaired) Total full LBM container extra space in bytes: 0 (0 repaired) Total incomplete LBM containers: 0 (0 repaired) Total LBM partial records: 0 (0 repaired) I0109 13:43:06.471539 160217 env_posix.cc:1676] Not raising this process' running threads per effective uid limit of 1033414; it is already as high as it can go I0109 13:43:06.479038 160217 ts_tablet_manager.cc:344] Loading tablet metadata (0/1989 complete) -- So the performance implications are limited. Below is the part of perf report of step 3: 1.99% data dir 9 [wor kudu-tserver.2 [.] kudu::fs::LogBlockManager::AddLogBlockUnlocked(scoped_refptr) ◆ 0.59% data dir 9 [wor [kernel.kallsyms][k] copy_user_enhanced_fast_string ?
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks The test waits for a write workload to generate some orphaned blocks. With the default write pattern, this could sometimes take a while and cause the test to hit an error. This patch makes orphaned block generation faster by making the workload update a single row. Before this, I looped the test in debug mode with 32 stress threads and it failed 15/100 times. With the fix, this passed 1000/1000. Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Reviewed-on: http://gerrit.cloudera.org:8080/12166 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-by: Mike Percy --- M src/kudu/integration-tests/ts_recovery-itest.cc 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-2195. Add additional gflag to force sync of consensus metadata
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12186 ) Change subject: KUDU-2195. Add additional gflag to force sync of consensus metadata .. Patch Set 1: Code-Review+2 Seems harmless enough. -- To view, visit http://gerrit.cloudera.org:8080/12186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73212d1670a33479cce7d9ef9ee61cfe9b00cdd3 Gerrit-Change-Number: 12186 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 09 Jan 2019 05:45:53 + Gerrit-HasComments: No
[kudu-CR] KUDU-2636: LBM supports deleting dead and full containers
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/12075 )
Change subject: KUDU-2636: LBM supports deleting dead and full containers
..
Patch Set 11:
(3 comments)
> And here are another three comments, may i fix them?
Go ahead. I agree with your first comment, disagree with the second, and don't
care either way on the third.
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@679
PS11, Line 679:
CONTAINER_DISK_FAILURE(block_manager_->env_->SyncDir(data_dir_->dir()),
> Still, i think it wastes IO to call 'SyncDir' after file deletion. The rogu
Now that we're calling SyncDir for each deleted container instead of just once
after Repair(), I agree.
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@2452
PS11, Line 2452: string data_filename = StrCat(container->ToString(),
kContainerDataFileSuffix);
: uint64_t reported_size;
: s = env_->GetFileSizeOnDisk(data_filename,
_size);
: if (!s.ok()) {
: HANDLE_DISK_FAILURE(s,
error_manager_->RunErrorNotificationCb(
: ErrorHandlerType::DISK_ERROR, dir));
: *result_status = s.CloneAndPrepend(Substitute(
: "Could not get on-disk file size of container $0",
container->ToString()));
: return;
: }
: int64_t cleanup_threshold_size =
container->live_bytes_aligned() *
: (1 +
FLAGS_log_container_excess_space_before_cleanup_fraction);
: if (reported_size > cleanup_threshold_size) {
:
local_report.full_container_space_check->entries.emplace_back(
: container->ToString(), reported_size -
container->live_bytes_aligned());
:
: // If the container is to be deleted outright, don't
bother repunching
: // its blocks. The report entry remains, however, so
it's clear that
: // there was a space discrepancy.
: if (container->live_blocks()) {
: need_repunching.insert(need_repunching.end(),
: dead_blocks.begin(),
dead_blocks.end());
: }
: }
> It's not necessary to clean up threshold size for a dead container, though
True, although the FsReport we generate tries to account for all
inconsistencies even if they're cleaned up by deleting containers. Plus
GetFileSizeOnDisk isn't particularly expensive.
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@2514
PS11, Line 2514: AddNewContainerUnlocked(container);
: MakeContainerAvailableUnlocked(std::move(container));
> It's a little bit strange to add the dead containers here and then remove t
Yeah but it's simpler in that all containers are treated the same way.
If you like, you could change the plumbing and avoid adding dead containers;
when I updated your patch it felt a little bit out of scope for me so I didn't
do it.
--
To view, visit http://gerrit.cloudera.org:8080/12075
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10
Gerrit-Change-Number: 12075
Gerrit-PatchSet: 11
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 09 Jan 2019 05:32:39 +
Gerrit-HasComments: Yes
[kudu-CR] tablet: clean up MergeIterState API
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12176 ) Change subject: tablet: clean up MergeIterState API .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Gerrit-Change-Number: 12176 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 05:33:05 + Gerrit-HasComments: No
[kudu-CR] rpc: add TCP socket statistics to /rpcz
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12184 ) Change subject: rpc: add TCP socket statistics to /rpcz .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12184/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/12184/1/src/kudu/rpc/reactor.h@152 PS1, Line 152: DumpRunningRpcs should probably rename this and related calls -- To view, visit http://gerrit.cloudera.org:8080/12184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I552c9dd80c0730ccd6bf7b13bb63761744a854c2 Gerrit-Change-Number: 12184 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 09 Jan 2019 04:35:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2636: LBM supports deleting dead and full containers
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/12075 )
Change subject: KUDU-2636: LBM supports deleting dead and full containers
..
Patch Set 11:
(3 comments)
Thank you very much :)
And here are another three comments, may i fix them?
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@679
PS11, Line 679:
CONTAINER_DISK_FAILURE(block_manager_->env_->SyncDir(data_dir_->dir()),
Still, i think it wastes IO to call 'SyncDir' after file deletion. The rogue
admin should be responsible for his operations. Besides, for the kudu newbee,
it will bring barriers while rebooting the kudu cluster. Anyway, that's only my
opinion.
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@2452
PS11, Line 2452: string data_filename = StrCat(container->ToString(),
kContainerDataFileSuffix);
: uint64_t reported_size;
: s = env_->GetFileSizeOnDisk(data_filename,
_size);
: if (!s.ok()) {
: HANDLE_DISK_FAILURE(s,
error_manager_->RunErrorNotificationCb(
: ErrorHandlerType::DISK_ERROR, dir));
: *result_status = s.CloneAndPrepend(Substitute(
: "Could not get on-disk file size of container $0",
container->ToString()));
: return;
: }
: int64_t cleanup_threshold_size =
container->live_bytes_aligned() *
: (1 +
FLAGS_log_container_excess_space_before_cleanup_fraction);
: if (reported_size > cleanup_threshold_size) {
:
local_report.full_container_space_check->entries.emplace_back(
: container->ToString(), reported_size -
container->live_bytes_aligned());
:
: // If the container is to be deleted outright, don't
bother repunching
: // its blocks. The report entry remains, however, so
it's clear that
: // there was a space discrepancy.
: if (container->live_blocks()) {
: need_repunching.insert(need_repunching.end(),
: dead_blocks.begin(),
dead_blocks.end());
: }
: }
It's not necessary to clean up threshold size for a dead container, though the
number of the dead containers is very limited(we know that the dead containers
will be deleted automatically at the runtime).
http://gerrit.cloudera.org:8080/#/c/12075/11/src/kudu/fs/log_block_manager.cc@2514
PS11, Line 2514: AddNewContainerUnlocked(container);
: MakeContainerAvailableUnlocked(std::move(container));
It's a little bit strange to add the dead containers here and then remove them
in the 'Repair()'. Maybe we could do something at Line2386.
--
To view, visit http://gerrit.cloudera.org:8080/12075
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10
Gerrit-Change-Number: 12075
Gerrit-PatchSet: 11
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 09 Jan 2019 03:22:22 +
Gerrit-HasComments: Yes
[kudu-CR] Reduce severity of "Error trying to read ahead of the log" log message
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12185 ) Change subject: Reduce severity of "Error trying to read ahead of the log" log message .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 Gerrit-Change-Number: 12185 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 02:44:38 + Gerrit-HasComments: No
[kudu-CR] tablet: clean up MergeIterState API
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12176 to look at the new patch set (#4). Change subject: tablet: clean up MergeIterState API .. tablet: clean up MergeIterState API This patch simply adds documentation and some readability cleanup to MergeIterState, which is a helper class for the MergeIterator. Non-API changes: - Add API docs to all public methods. - Extract the implementation of Advance() and PullNextBlock() out of the class declaration to make the API contract easier to quickly read and understand. - Rename a couple of private variables to clarify row-oriented semantics. API cleanup: - Add a public Init() method that delegates to PullNextBlock(). - Make PullNextBlock() and IsBlockExhausted() private methods. - Replace exposure of the underlying iterator via iter() with a public AddStats() method, since stats collection was the only use of iter(). There are no functional changes in this patch. Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 --- M src/kudu/common/generic_iterators.cc 1 file changed, 89 insertions(+), 64 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/12176/4 -- To view, visit http://gerrit.cloudera.org:8080/12176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Gerrit-Change-Number: 12176 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] tablet: clean up MergeIterState API
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12176 ) Change subject: tablet: clean up MergeIterState API .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12176/3/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12176/3/src/kudu/common/generic_iterators.cc@126 PS3, Line 126: // Exposed for optimization purposes. > I don't understand this part. We use remaining_in_block() to resize the cli Oops, I thought it was just a reservation. I'll just remove this line. -- To view, visit http://gerrit.cloudera.org:8080/12176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Gerrit-Change-Number: 12176 Gerrit-PatchSet: 3 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 01:52:02 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2195. Add additional gflag to force sync of consensus metadata
Hello Alexey Serbin, Adar Dembo, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12186 to review the following change. Change subject: KUDU-2195. Add additional gflag to force sync of consensus metadata .. KUDU-2195. Add additional gflag to force sync of consensus metadata This patch adds an override gflag for consensus metadata fsync so that XFS users are less likely to lose their consensus metadata files while voting right before a power outage. Change-Id: I73212d1670a33479cce7d9ef9ee61cfe9b00cdd3 --- M src/kudu/consensus/consensus_meta.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/12186/1 -- To view, visit http://gerrit.cloudera.org:8080/12186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I73212d1670a33479cce7d9ef9ee61cfe9b00cdd3 Gerrit-Change-Number: 12186 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 00:45:53 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations warning on macOS
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12183 ) Change subject: [gutil] suppress -Wdeprecated-declarations warning on macOS .. [gutil] suppress -Wdeprecated-declarations warning on macOS As it turned out, most of the atomic functions used in atomicops-internals-macosx.h have been declared deprecated since macOS 10.12, most of the functions in the SASL API are deprecated since macOS 10.11, and the krb5 API is deprecated in favor of GSS.framework. To avoid multiple compilation warnings on macOS while compiling with clang from the Xcode development environment, this changelist adds 'pragma GCC diagnostic push/pop' for the '-Wdeprecated-declarations' flag accordingly. It's confirmed that this patch suppresses deprecation warnings at least on macOS 10.14. This is a follow-up to 678dbac6fb05d0370e40e6645d4b1ec530fa0180. Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Reviewed-on: http://gerrit.cloudera.org:8080/12183 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/gutil/atomicops-internals-macosx.h M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc M src/kudu/security/init.cc M src/kudu/thrift/sasl_client_transport.cc 8 files changed, 85 insertions(+), 0 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2195 (part 1): always sync PBC-format metadata files
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9043 ) Change subject: KUDU-2195 (part 1): always sync PBC-format metadata files .. Patch Set 1: This was recently seen in the wild again. Usually it's people running XFS that experience a power outage who see 0-length cmeta files. We should consider adding a gflag for just the cmeta files so that people running with XFS have a band-aid. -- To view, visit http://gerrit.cloudera.org:8080/9043 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4f0c911662b2ff35fcf3915790248ba85bf6026f Gerrit-Change-Number: 9043 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 09 Jan 2019 00:38:21 + Gerrit-HasComments: No
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 00:23:33 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations warning on macOS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12183 ) Change subject: [gutil] suppress -Wdeprecated-declarations warning on macOS .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 09 Jan 2019 00:27:23 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations warning on macOS
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12183 to look at the new patch set (#3). Change subject: [gutil] suppress -Wdeprecated-declarations warning on macOS .. [gutil] suppress -Wdeprecated-declarations warning on macOS As it turned out, most of the atomic functions used in atomicops-internals-macosx.h have been declared deprecated since macOS 10.12, most of the functions in the SASL API are deprecated since macOS 10.11, and the krb5 API is deprecated in favor of GSS.framework. To avoid multiple compilation warnings on macOS while compiling with clang from the Xcode development environment, this changelist adds 'pragma GCC diagnostic push/pop' for the '-Wdeprecated-declarations' flag accordingly. It's confirmed that this patch suppresses deprecation warnings at least on macOS 10.14. This is a follow-up to 678dbac6fb05d0370e40e6645d4b1ec530fa0180. Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 --- M src/kudu/gutil/atomicops-internals-macosx.h M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc M src/kudu/security/init.cc M src/kudu/thrift/sasl_client_transport.cc 8 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/12183/3 -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Reduce severity of "Error trying to read ahead of the log" log message
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12185 ) Change subject: Reduce severity of "Error trying to read ahead of the log" log message .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 Gerrit-Change-Number: 12185 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Wed, 09 Jan 2019 00:23:59 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12183 ) Change subject: [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 .. Patch Set 2: Code-Review+2 Confirmed it eliminates the warnings on macOS 10.14. -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 09 Jan 2019 00:15:06 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12183 to look at the new patch set (#2). Change subject: [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 .. [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 As it turned out, most of the atomic operations functions declared in atomicops-internals-macosx.h have been declared deprecated since macOS 10.12, most of the functions in the SASL API are deprecated since macOS 10.11, and krb5 API is deprecated in favor to GSS.framework. This changelist adds 'pragma GCC diagnostic push/pop' for the '-Wdeprecated-declarations' flag accordingly. This is a follow-up to 678dbac6fb05d0370e40e6645d4b1ec530fa0180. Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 --- M src/kudu/gutil/atomicops-internals-macosx.h M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc M src/kudu/security/init.cc M src/kudu/thrift/sasl_client_transport.cc 8 files changed, 83 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/12183/2 -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Reduce severity of "Error trying to read ahead of the log" log message
Hello Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12185 to look at the new patch set (#2). Change subject: Reduce severity of "Error trying to read ahead of the log" log message .. Reduce severity of "Error trying to read ahead of the log" log message In clusters under load, it is typical to see the ERROR log dominated by messages like the following: E1221 09:09:13.869918 143384 consensus_queue.cc:440] T 1d030514317942ec9d7796a8a029dace P a4eea0affa4545879c8988b24d8cb2bb [LEADER]: Error trying to read ahead of the log while preparing peer request: Incomplete: Op with index 6620 is ahead of the local log (next sequential op: 6620). Destination peer: Peer: c0a2e34b708845efb8f090459815272c, Is new: false, Last received: 2995.6619, Next index: 6621, Last known committed idx: 6620, Last exchange result: ERROR, Needs tablet copy: false This message is logged at the error level, and looks a little bit scary, but it is in fact harmless. Here's what happens: 1. A leader neglects its duties and the followers elect a new leader. 2. The new leader manages to replicate more ops (usually just the NO_OP asserting leadership). 3. The leader of the previous term tries to replicate an op to a peer in the new term. 4. From the response, it founds out that a. It is in an earlier term, so it should step down and increment its term. b. The last op the peer saw is (leader's index + k) for some k > 0 (usually k = 1). So the leader will attempt to send up ops of index up through (leader's index + k). 5. The term change is asynchronous, and before it happens the leader tries to prepare a new request to the peer whose log is ahead of the local log. This causes the ERROR message. 6. The term change happens. The leader steps down, and life goes on. Note that the leader should also have received a VoteRequest with the new term and an UpdateConsensus with the new term from the leader. In general, this message appears only when the leader is under enough stress to lose its leadership and be unable to service some consensus RPCs in a timely manner. It is not in an of itself a problem, and it's a decent indicator of stress on the leader, so I am leaving the message but downgrading it to INFO level. See KUDU-1078 for more information about this situation, especially its previous causes (which were at one time actually harmful). Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 --- M src/kudu/consensus/consensus_queue.cc 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/12185/2 -- To view, visit http://gerrit.cloudera.org:8080/12185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 Gerrit-Change-Number: 12185 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] Reduce severity of "Error trying to read ahead of the log" log message
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12185 Change subject: Reduce severity of "Error trying to read ahead of the log" log message .. Reduce severity of "Error trying to read ahead of the log" log message In clusters under load, it is typical to see the ERROR log dominated by messages like the following: E1221 09:09:13.869918 143384 consensus_queue.cc:440] T 1d030514317942ec9d7796a8a029dace P a4eea0affa4545879c8988b24d8cb2bb [LEADER]: Error trying to read ahead of the log while preparing peer request: Incomplete: Op with index 6620 is ahead of the local log (next sequential op: 6620). Destination peer: Peer: c0a2e34b708845efb8f090459815272c, Is new: false, Last received: 2995.6619, Next index: 6621, Last known committed idx: 6620, Last exchange result: ERROR, Needs tablet copy: false This message is logged at the error level, and looks a little bit scary, but it is in fact harmless. Here's what happens: 1. A leader neglects its duties and the followers elect a new leader. 2. The new leader manages to replicate more ops (usually just the NO_OP asserting leadership). 3. The leader of the previous term tries to replicate an op to a peer in the new term. 4. From the response, it founds out that a. It is in an earlier term, so it should step down and increment its term. b. The next op the peer expects is (leader's index + k) for some k > 0 (usually k = 1). 5. The term change is asynchronous, and before it happens the leader tries to prepare a new request to the peer whose log is ahead of the local log. This causes the ERROR message. 6. The term change happens. The leader steps down, and life goes on. Note that the leader should also have received a VoteRequest with the new term and an UpdateConsensus with the new term from the leader. In general, this message appears only when the leader is under enough stress to lose its leadership and be unable to service some consensus RPCs in a timely manner. It is not in an of itself a problem, and it's a decent indicator of stress on the leader, so I am leaving the message but downgrading it to INFO level. See KUDU-1078 for more information about this situation, especially its previous causes (which were at one time actually harmful). Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 --- M src/kudu/consensus/consensus_queue.cc 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/12185/1 -- To view, visit http://gerrit.cloudera.org:8080/12185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib841a52354e5c71450f3e364ab2fdee51ce2adb4 Gerrit-Change-Number: 12185 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG@11 PS1, Line 11: This patch makes orphaned block : generation faster by making the workload update a single row. > I would also suspect Will's compaction changes; I wonder if flushing outstr >Hmm, why is this obviously faster? The default is a random write workload, no a sequential one. It's a bit harder to reason about what the rowsets look like, but I'd expect either sequential writes or single-row-updates to definitely generate orphaned blocks. >I also wonder why this surfaced just now. I actually just saw this fail in a Cloudera-internal 1.8 build, so I don't think the flakiness was introduced by the recent compaction policy changes. http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc@257 PS1, Line 257: // We want to generate orphaned blocks, so use a workload that we expect to > I think the explanation is that we end up generating lots of (major and/or Done, yes. -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 08 Jan 2019 23:48:40 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Hello Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12166 to look at the new patch set (#2). Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks The test waits for a write workload to generate some orphaned blocks. With the default write pattern, this could sometimes take a while and cause the test to hit an error. This patch makes orphaned block generation faster by making the workload update a single row. Before this, I looped the test in debug mode with 32 stress threads and it failed 15/100 times. With the fix, this passed 1000/1000. Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c --- M src/kudu/integration-tests/ts_recovery-itest.cc 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/12166/2 -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG@11 PS1, Line 11: This patch makes orphaned block : generation faster by making the workload update a single row. > Hmm, why is this obviously faster? Updating one row over and over means we' I would also suspect Will's compaction changes; I wonder if flushing outstrips the merge compactions in the old scenario now. -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 08 Jan 2019 23:40:57 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12166 ) Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc@257 PS1, Line 257: write_workload->set_write_pattern(TestWorkload::UPDATE_ONE_ROW); > Should add a comment here explaining why we're using this. I think the explanation is that we end up generating lots of (major and/or minor) delta compactions, right? -- To view, visit http://gerrit.cloudera.org:8080/12166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c Gerrit-Change-Number: 12166 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 08 Jan 2019 23:39:23 + Gerrit-HasComments: Yes
[kudu-CR] rpc: add TCP socket statistics to /rpcz
Hello Michael Ho, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12184 to review the following change. Change subject: rpc: add TCP socket statistics to /rpcz .. rpc: add TCP socket statistics to /rpcz This adds the ability to fetch various bits of socket-level information for each RPC connection and publish the info into /rpcz. The information itself is fetched using getsockopt(TCP_INFO) as well as ioctls to check the current send and receive queue lengths. This data can help resolve whether a use case is network bound or bound by the application itself. For example, a high number of retransmitted packets can indicate that the network path to the receiver is overloaded. Eventually we may want to expose some of this information on a per-call basis. However, doing so is quite tricky, since 'send()' completes when the data has been placed into the outbound packet queue and doesn't wait until the data is ACKed. We'd need to defer checking for retransmissions until all of the data has been ACKed, which is at some indeterminate point in the future. The very newest kernels allow subscribing to such notifications (along with lots of interesting stats) but, given none of that is available in el7, it's probably not worth tackling at this point. Change-Id: I552c9dd80c0730ccd6bf7b13bb63761744a854c2 --- M src/kudu/rpc/connection.cc M src/kudu/rpc/connection.h M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/inbound_call.h M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/outbound_call.cc M src/kudu/rpc/outbound_call.h M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc_introspection.proto M src/kudu/rpc/rpc_stub-test.cc M src/kudu/server/rpcz-path-handler.cc 14 files changed, 299 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/12184/1 -- To view, visit http://gerrit.cloudera.org:8080/12184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I552c9dd80c0730ccd6bf7b13bb63761744a854c2 Gerrit-Change-Number: 12184 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Michael Ho
[kudu-CR] [test] dos2unix for rowset tree-test
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12182 ) Change subject: [test] dos2unix for rowset_tree-test .. [test] dos2unix for rowset_tree-test This patch converts DOS-style line breaks into Unix-like ones in src/kudu/tablet/rowset_tree-test.cc This changelist does not contain any functional changes. Change-Id: I212a5523f48bf305d53e471300d20b7eed932deb Reviewed-on: http://gerrit.cloudera.org:8080/12182 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M src/kudu/tablet/rowset_tree-test.cc 1 file changed, 38 insertions(+), 38 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I212a5523f48bf305d53e471300d20b7eed932deb Gerrit-Change-Number: 12182 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12183 ) Change subject: [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 23:01:59 + Gerrit-HasComments: No
[kudu-CR] [test] dos2unix for rowset tree-test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12182 ) Change subject: [test] dos2unix for rowset_tree-test .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I212a5523f48bf305d53e471300d20b7eed932deb Gerrit-Change-Number: 12182 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 23:00:29 + Gerrit-HasComments: No
[kudu-CR] [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12183 Change subject: [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 .. [gutil] suppress -Wdeprecated-declarations on macOS >= 10.12 As it turned out, most of the atomic operations functions declared in atomicops-internals-macosx.h have been declared deprecated at at 10.12 and higher. This changelist adds 'pragma GCC diagnostic push/pop' for the '-Wdeprecated-declarations' flag accordingly. This is a follow-up to 678dbac6fb05d0370e40e6645d4b1ec530fa0180. Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 --- M src/kudu/gutil/atomicops-internals-macosx.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/12183/1 -- To view, visit http://gerrit.cloudera.org:8080/12183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I82b0ebd9917a567d1a8b72c80b47dc102304d860 Gerrit-Change-Number: 12183 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [tools] Add table scan tool
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/12167 )
Change subject: [tools] Add table scan tool
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc@81
PS4, Line 81: '[', '(', '=', ')' or ']'"
> I find these really confusing. For example, in the sample from the help mes
yea, I was thinking the same thing -- rather than inventing a new syntax, I
think a JSON representation of a parsed expression (AST-alike) would be much
easier to reason about. Plus, we don't need to worry about implementing a
parser :)
For example, ["and", ["<", "key", 123], ["notnull", "col2"]]. It sort of ends
up being a lisp dialect, proving out Greenspun's tenth rule
--
To view, visit http://gerrit.cloudera.org:8080/12167
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieac340b70a9eaf131f82a2b7d61336211d1d48f8
Gerrit-Change-Number: 12167
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Tue, 08 Jan 2019 22:22:43 +
Gerrit-HasComments: Yes
[kudu-CR] [test] dos2unix for rowset tree-test
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12182 Change subject: [test] dos2unix for rowset_tree-test .. [test] dos2unix for rowset_tree-test This patch converts DOS-style line breaks into Unix-like ones in src/kudu/tablet/rowset_tree-test.cc This changelist does not contain any functional changes. Change-Id: I212a5523f48bf305d53e471300d20b7eed932deb --- M src/kudu/tablet/rowset_tree-test.cc 1 file changed, 38 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/12182/1 -- To view, visit http://gerrit.cloudera.org:8080/12182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I212a5523f48bf305d53e471300d20b7eed932deb Gerrit-Change-Number: 12182 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12178 ) Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 22:20:31 + Gerrit-HasComments: No
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12178 to look at the new patch set (#4). Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. KUDU-2656: pass IOContext to ValidateDeltaOrder Previously, checksum errors that occurred during calls to the debug-only function ValidateDeltaOrder() would lead to a DCHECK failure. This patch plumbs an IOContext to these calls to avoid this and adds a regression test for such cases. Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b --- M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 6 files changed, 60 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/12178/4 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12178 to look at the new patch set (#3). Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. KUDU-2656: pass IOContext to ValidateDeltaOrder Previously, checksum errors that occurred during calls to the debug-only function ValidateDeltaOrder() would lead to a DCHECK failure. This patch plumbs an IOContext to these calls to avoid this and adds a regression test for such cases. Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b --- M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 6 files changed, 59 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/12178/3 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/12178 )
Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/delta_tracker.cc@207
PS2, Line 207: #ifndef NDEBUG
: Status DeltaTracker::ValidateDeltaOrder(const
std::shared_ptr& first,
: const
std::shared_ptr& second,
: const IOContext*
io_context,
: DeltaType type) {
: shared_ptr first_copy = first;
: shared_ptr second_copy = second;
:
: // Make clones so we don't leave the original ones initted.
That can affect
: // tests. We know it's a DeltaFileReader if it's not Initted().
: if (!first_copy->Initted()) {
: shared_ptr first_clone;
:
RETURN_NOT_OK(down_cast(first.get())->CloneForDebugging(
: rowset_metadata_->fs_manager(),
mem_trackers_.tablet_tracker, _clone));
: RETURN_NOT_OK(first_clone->Init(io_context));
: first_copy = first_clone;
: }
: if (!second_copy->Initted()) {
: shared_ptr second_clone;
:
RETURN_NOT_OK(down_cast(second.get())->CloneForDebugging(
: rowset_metadata_->fs_manager(),
mem_trackers_.tablet_tracker, _clone));
: RETURN_NOT_OK(second_clone->Init(io_context));
: second_copy = second_clone;
: }
:
: switch (type) {
: case REDO:
: DCHECK_LE(first_copy->delta_stats().min_timestamp(),
: second_copy->delta_stats().min_timestamp())
: << "Found out-of-order deltas: [{" <<
first_copy->ToString() << "}, {"
: << second_copy->ToString() << "}]: type = " << type;
: break;
: case UNDO:
: DCHECK_GE(first_copy->delta_stats().min_timestamp(),
: second_copy->delta_stats().min_timestamp())
: << "Found out-of-order deltas: [{" <<
first_copy->ToString() << "}, {"
: << second_copy->ToString() << "}]: type = " << type;
: break;
: }
: return Status::OK();
: }
:
: Status DeltaTracker::ValidateDeltasOrdered(const
SharedDeltaStoreVector& list,
:const IOContext*
io_context,
:DeltaType type) {
: for (size_t i = 1; i < list.size(); i++) {
: RETURN_NOT_OK(ValidateDeltaOrder(list[i - 1], list[i],
io_context, type));
: }
:
> If this is not used elsewhere but only in ifndef NDEBUG case, maybe put tho
Done
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc
File src/kudu/tablet/major_delta_compaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@186
PS2, Line 186: const
> nit: constexpr ?
Done
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@191
PS2, Line 191: .empty());
> paranoid nit: is it guaranteed that all_rowsets container is non-empty?
It should be guaranteed because we've written to the tablet, but I'll add one
anyway.
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@205
PS2, Line 205: Status s = tablet()->DoMajorDeltaCompaction(col_ids, rs,
_context);
> nit: maybe, add ' << s.ToString(); for easier debugging if that fails
Done
--
To view, visit http://gerrit.cloudera.org:8080/12178
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b
Gerrit-Change-Number: 12178
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 22:11:32 +
Gerrit-HasComments: Yes
[kudu-CR](gh-pages) [blog] a blogpost about location awareness in Kudu
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12119 ) Change subject: [blog] a blogpost about location awareness in Kudu .. Patch Set 2: (21 comments) http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md File _posts/2018-12-20-location-awareness.md: http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@70 PS2, Line 70: a http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@98 PS2, Line 98: a http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@112 PS2, Line 112: when where http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@114 PS2, Line 114: is are http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@115 PS2, Line 115: a the http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@117 PS2, Line 117: the http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@117 PS2, Line 117: kudu Either Kudu, or put in backticks `kudu`. http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@121 PS2, Line 121: very Remove. http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@121 PS2, Line 121: At In http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@122 PS2, Line 122: policies policy http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@122 PS2, Line 122: re-establishes reestablish http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@124 PS2, Line 124: continue to the cross-location rebalancing. Add "phase" at the end. http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@126 PS2, Line 126: Second phase is cross-location rebalancing, i.e. moving tablet replicas between "The second phase" http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@127 PS2, Line 127: location locations http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@128 PS2, Line 128: equalizing loads of locations throughout the cluster. Use the the http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@128 PS2, Line 128: loads What is the load of a location? http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@132 PS2, Line 132: replica replicas http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@132 PS2, Line 132: Third phase is intra-location rebalancing, i.e. balance tablet replica The http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@133 PS2, Line 133: such the http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@133 PS2, Line 133: distribution Remove. http://gerrit.cloudera.org:8080/#/c/12119/2/_posts/2018-12-20-location-awareness.md@135 PS2, Line 135: A very quick example showing the moves for each of the three phases would be good. It would illustrate the three phases and the role of "load" in cross-location rebalancing. -- To view, visit http://gerrit.cloudera.org:8080/12119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I10b30a80d5661fb889a11285b8118cdea1a87cd2 Gerrit-Change-Number: 12119 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 21:40:44 + Gerrit-HasComments: Yes
[kudu-CR] [tools] Add table scan tool
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/12167 )
Change subject: [tools] Add table scan tool
..
Patch Set 4:
(5 comments)
I left a few high-level comments, but at the top level: what is the goal of
this tool? Is it to serve as a barebones shell, like impala-shell or MySQL
shell? Is it meant for benchmarking scan performance?
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/kudu-tool-test.cc@2316
PS4, Line 2316: TEST_F(ToolTest, TestScanTable)
This requires a lot more tests. Correctness tests should cover at least the
following things:
1. Scanning with no predicates and no columns for just a row count (Done).
2. Scanning with each predicate and checking it gets exactly the correct rows
(includes matching, excludes non matching).
3. Scanning with projections of one or more columns and making sure the right
columns are returned.
4. Scanning with multiple predicates on one column.
5. Scanning with multiple predicates across different columns.
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc@18
PS4, Line 18: #include
Prefer #include .
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc@76
PS4, Line 76: "Query predicates on columns. Unlike the
traditional"
Here and elsewhere, add "\n" to include linebreaks in the help message.
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc@79
PS4, Line 79: WhetherNull
I think we can call this IsNull. It's shorter.
http://gerrit.cloudera.org:8080/#/c/12167/4/src/kudu/tools/tool_action_table.cc@81
PS4, Line 81: '[', '(', '=', ')' or ']'"
I find these really confusing. For example, in the sample from the help message:
col1:[:lower;col1:]:upper;col2:@:v1,v2,v3;col3:n:NULL
My brain matches up [] pairs and wants to group together what's between them,
so I want to interpret '[:lower;col1:]' as something meaningful when it's split
between two predicates. Could we use friendlier symbols for the comparison
operators? Can we just use < for <, <= for <=, etc?
Also, a few other things we ought to try and address with the syntax (or maybe
just with documentation and help messages):
1. How can a user query for, e.g. 'KEY <= 10 AND KEY > 0'? Use two predicates?
2. Could we use = for equality and in-list predicates? E.g. 'col0:=:5' and
'col0:=:5,6,7,8'?
3. Why do we need the ":NULL" at the end of 'col3:n:NULL'? It's easier to
parse, for sure, but it conveys no meaning.
4. How would a user do an equality predicate on a string with a comma in it?
I think it might be a good idea to use a simple JSON syntax for the predicates.
This gives us a defined way to handle all the different characters, at the cost
of some verbosity.
--
To view, visit http://gerrit.cloudera.org:8080/12167
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieac340b70a9eaf131f82a2b7d61336211d1d48f8
Gerrit-Change-Number: 12167
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Tue, 08 Jan 2019 21:30:41 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2636: LBM supports deleting dead and full containers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12075 ) Change subject: KUDU-2636: LBM supports deleting dead and full containers .. Patch Set 11: I pushed a new revision with a few cleanups: - Properly rebased on top of the "wrapped up containers in scoped_refptr" change. - The few remaining DOS line endings converted to UNIX line endings. - A few formatting/style fixes. - The only real change was to get read_only handling right: in that case, we should not delete dead containers at startup. To do this I moved the call to TrySetDead into Repair(), and changed the container vector plumbing from OpenDataDir() into Repair() to pass containers by scoped_refptr instead of by name. Please take a look and let me know if anything seems wrong. -- To view, visit http://gerrit.cloudera.org:8080/12075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10 Gerrit-Change-Number: 12075 Gerrit-PatchSet: 11 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 08 Jan 2019 20:36:23 + Gerrit-HasComments: No
[kudu-CR] KUDU-2636: LBM supports deleting dead and full containers
Adar Dembo has uploaded a new patch set (#11) to the change originally created by helifu. ( http://gerrit.cloudera.org:8080/12075 ) Change subject: KUDU-2636: LBM supports deleting dead and full containers .. KUDU-2636: LBM supports deleting dead and full containers LBM supports deleting the full containers which are dead before hole punching. It can help to ease disk pressure, save tserver startup time and etc. It uses the container size and live blocks to determine whether the container is dead or live. Step1: wrapping up containers in scoped_refptr. [V] Step2: supporting dead containers removal. [V] Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10 --- M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h 3 files changed, 235 insertions(+), 121 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/12075/11 -- To view, visit http://gerrit.cloudera.org:8080/12075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10 Gerrit-Change-Number: 12075 Gerrit-PatchSet: 11 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu
[kudu-CR] [fs]: wrapping up containers in scoped refptr
Adar Dembo has uploaded a new patch set (#6) to the change originally created by helifu. ( http://gerrit.cloudera.org:8080/12121 ) Change subject: [fs]: wrapping up containers in scoped_refptr .. [fs]: wrapping up containers in scoped_refptr It's necessary to wrap up containers in scoped_refptr to support deleting the full containers that are dead at runtime. Based on this, the KUDU-2636 can be fixed. Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 --- M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h 3 files changed, 129 insertions(+), 141 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/12121/6 -- To view, visit http://gerrit.cloudera.org:8080/12121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c5c620014782b3d57dcbe047d0df73c949590c7 Gerrit-Change-Number: 12121 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu
[kudu-CR] Fix DOS line endings in TestServerInfo.java
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12180 ) Change subject: Fix DOS line endings in TestServerInfo.java .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12180/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: PS1: > In my case, the dos2unix tool doesn't see those neither. I'm curious what This turned out to be an artifact of how the "new" gerrit UI works in this version of gerrit. -- To view, visit http://gerrit.cloudera.org:8080/12180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb Gerrit-Change-Number: 12180 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 20:20:08 + Gerrit-HasComments: Yes
[kudu-CR] Fix DOS line endings in TestServerInfo.java
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12180 ) Change subject: Fix DOS line endings in TestServerInfo.java .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/12180/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: PS1: > Where do you see them? I don't see them in vim, I don't see them in Intelli In my case, the dos2unix tool doesn't see those neither. I'm curious what else it might be. -- To view, visit http://gerrit.cloudera.org:8080/12180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb Gerrit-Change-Number: 12180 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 19:58:17 + Gerrit-HasComments: Yes
[kudu-CR] Fix DOS line endings in TestServerInfo.java
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12180 ) Change subject: Fix DOS line endings in TestServerInfo.java .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12180/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: PS1: > Maybe gerrit is doing a bad job of visualizing this, but it looks like some Where do you see them? I don't see them in vim, I don't see them in IntelliJ with "show whitespace" on. -- To view, visit http://gerrit.cloudera.org:8080/12180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb Gerrit-Change-Number: 12180 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 19:41:12 + Gerrit-HasComments: Yes
[kudu-CR] Fix DOS line endings in TestServerInfo.java
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12180 ) Change subject: Fix DOS line endings in TestServerInfo.java .. Patch Set 1: (1 comment) Maybe http://gerrit.cloudera.org:8080/#/c/12180/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: PS1: Maybe gerrit is doing a bad job of visualizing this, but it looks like some lines still have a second line end character. -- To view, visit http://gerrit.cloudera.org:8080/12180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb Gerrit-Change-Number: 12180 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 19:07:49 + Gerrit-HasComments: Yes
[kudu-CR] Assign locations to tablet servers and the client in Java
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12174 ) Change subject: Assign locations to tablet servers and the client in Java .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/12174/4/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java: http://gerrit.cloudera.org:8080/#/c/12174/4/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@624 PS4, Line 624: * Adds location info that will configure how locations are assigned to : * tablet servers and clients. Location strings should be in the form : * 'location:number'. For example, a location info like : * ["/L0:2", "/L1:1", "/L2:1"] : * will assign locations to a total of four clients or tablet servers. : * 2 tablet servers or clients will get the location '/L0' while one : * will get location '/L1' and one will get location '/L2'. This should probably be rewritten to account for the new 'one location at a time' style. -- To view, visit http://gerrit.cloudera.org:8080/12174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e2c74ab12f7da187bf6e75d42a3089bc20235db Gerrit-Change-Number: 12174 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 19:06:17 + Gerrit-HasComments: Yes
[kudu-CR] Support location awareness in READ CLOSEST for the Java client
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12175 ) Change subject: Support location awareness in READ_CLOSEST for the Java client .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief0f07058cefd0037f4b0f7c60c8b7809dc8313f Gerrit-Change-Number: 12175 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 19:06:59 + Gerrit-HasComments: No
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 ) Change subject: Support location awareness in READ_CLOSEST for the C++ client .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 8 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 19:05:00 + Gerrit-HasComments: No
[kudu-CR] KUDU-2348: In the Java client, pick a random replica when no replica is local
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12158 ) Change subject: KUDU-2348: In the Java client, pick a random replica when no replica is local .. Patch Set 8: My bad, had the wrong link in that last comment. I meant this changelist: https://gerrit.cloudera.org/#/c/12175/. -- To view, visit http://gerrit.cloudera.org:8080/12158 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d70e45d4c9532bb32223c10936b4ff8fd99 Gerrit-Change-Number: 12158 Gerrit-PatchSet: 8 Gerrit-Owner: Zhang Yifan Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: Zhang Yifan Gerrit-Comment-Date: Tue, 08 Jan 2019 19:02:44 + Gerrit-HasComments: No
[kudu-CR] KUDU-2348: In the Java client, pick a random replica when no replica is local
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12158 ) Change subject: KUDU-2348: In the Java client, pick a random replica when no replica is local .. Patch Set 8: You might also want to take a look at https://gerrit.cloudera.org/#/c/11088/, which is changing the CLOSEST_REPLICA logic to also take into account locations (i.e. rack awareness / rack assignments). -- To view, visit http://gerrit.cloudera.org:8080/12158 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3d70e45d4c9532bb32223c10936b4ff8fd99 Gerrit-Change-Number: 12158 Gerrit-PatchSet: 8 Gerrit-Owner: Zhang Yifan Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Reviewer: Zhang Yifan Gerrit-Comment-Date: Tue, 08 Jan 2019 19:01:51 + Gerrit-HasComments: No
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Hello Tidy Bot, Alexey Serbin, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12138 to look at the new patch set (#8). Change subject: Support location awareness in READ_CLOSEST for the C++ client .. Support location awareness in READ_CLOSEST for the C++ client Previously, in READ_CLOSEST, the C++ client would choose to read from a local tablet server if possible and otherwise pick a random replica. This changes and enhances READ_CLOSEST mode to work as follows: 1. If there is a local server, use it. If there are multiple local servers, choose one at random. 2. If there is a server in the same location as the client, use it. If there are multiple servers in the same location, choose one at random. 3. Otherwise, choose a server at random. This is not a breaking change, as in the absence of locations the behavior is consistent with what was documented before, which was only that a local server would be chosen if possible, else a random server. Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/integration-tests/location_assignment-itest.cc 7 files changed, 150 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/12138/8 -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 8 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Fix DOS line endings in TestServerInfo.java
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12180 Change subject: Fix DOS line endings in TestServerInfo.java .. Fix DOS line endings in TestServerInfo.java Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java 1 file changed, 81 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/12180/1 -- To view, visit http://gerrit.cloudera.org:8080/12180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id0c470ed24238e0461e96bd443f3d649e1e11acb Gerrit-Change-Number: 12180 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] Assign locations to tablet servers and the client in Java
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12174 to look at the new patch set (#4). Change subject: Assign locations to tablet servers and the client in Java .. Assign locations to tablet servers and the client in Java Change-Id: I9e2c74ab12f7da187bf6e75d42a3089bc20235db --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java A java/kudu-client/src/test/resources/assign-location.py M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java M java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java 9 files changed, 152 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/12174/4 -- To view, visit http://gerrit.cloudera.org:8080/12174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9e2c74ab12f7da187bf6e75d42a3089bc20235db Gerrit-Change-Number: 12174 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Support location awareness in READ CLOSEST for the Java client
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12175 to look at the new patch set (#3). Change subject: Support location awareness in READ_CLOSEST for the Java client .. Support location awareness in READ_CLOSEST for the Java client Change-Id: Ief0f07058cefd0037f4b0f7c60c8b7809dc8313f --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/ReplicaSelection.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java M java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java 7 files changed, 141 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/12175/3 -- To view, visit http://gerrit.cloudera.org:8080/12175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ief0f07058cefd0037f4b0f7c60c8b7809dc8313f Gerrit-Change-Number: 12175 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] Support location awareness in READ CLOSEST for the Java client
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/12175 )
Change subject: Support location awareness in READ_CLOSEST for the Java client
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@520
PS2, Line 520: private synchronized String getLocation() {
> I don't think you need this. Access to 'location' doesn't need to be synchr
Done
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@220
PS2, Line 220:* @param replicaSelection replica selection mechanism to use
> Update the @param list.
Done
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java:
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@105
PS2, Line 105: location
> Nit: maybe shorten to 'loc' to avoid the disambiguation below?
Done
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
File
java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java:
http://gerrit.cloudera.org:8080/#/c/12175/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java@218
PS2, Line 218: static RemoteTablet getTablet(int leaderIndex, int
localReplicaIndex, int sameLocationReplicaIndex) {
> Nit: line too long?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12175
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief0f07058cefd0037f4b0f7c60c8b7809dc8313f
Gerrit-Change-Number: 12175
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Tue, 08 Jan 2019 18:58:13 +
Gerrit-HasComments: Yes
[kudu-CR] Assign locations to tablet servers and the client in Java
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/12174 )
Change subject: Assign locations to tablet servers and the client in Java
..
Patch Set 3:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1884
PS3, Line 1884: final String location = tsInfoPB.getLocation();
: return new ServerInfo(uuid, hostPort, inetAddress,
location);
> Nit: combine
Done
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@49
PS3, Line 49: --
> nit: ' --' or replace with ';'
Done
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@99
PS3, Line 99: public String getLocation() { return location; }
> Nit: I don't think we use this "cram it all on one line" style in Java.
The IDE was visualizing the getters in the file as crammed on one line so I
thought that was the style for them :(.
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1123
PS3, Line 1123: assertEquals("", client.asyncClient.location);
> Not much of a test though because this assert is true before the listTablet
True, but it still tests that nothing changes by connecting to the master.
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java:
PS3:
> I guess the very first version of this file was had DOS end-of-line symbols
I'll do that in a follow up. I have to redo the whole change if I do it prior
to these changes since git calls the entire contents of the file as a conflict.
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java@1
PS3, Line 1: /**
: * Licensed under the Apache License, Version 2.0 (the "License");
: * you may not use this file except in compliance with the License.
: * You may obtain a copy of the License at
: *
: * http://www.apache.org/licenses/LICENSE-2.0
: *
: * Unless required by applicable law or agreed to in writing,
software
: * distributed under the License is distributed on an "AS IS"
BASIS,
: * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
: * See the License for the specific language governing permissions
and
: * limitations under the License. See accompanying LICENSE file.
: */
> nit: while you are at it, maybe unify the license header and make it commen
Done
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/resources/assign-location.py
File java/kudu-client/src/test/resources/assign-location.py:
PS3:
> Can you symlink this to the existing assign-location.py rather than copy it
Seems like it works.
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
File
java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@229
PS3, Line 229: getClass().getResource("/assign-location.py").getFile()
> I'm curious whether this works when tests are run using dist-test.
The precommit ran this test using dist-test, so it does!
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@230
PS3, Line 230: String locationMappingCmdPath = Paths.get(clusterRoot,
"/location-assignment.state").toString();
> Nit: too long?
Done
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@633
PS3, Line 633: public MiniKuduClusterBuilder
addLocationInfo(Collection locations) {
> Would it be more ergonomic if this added one location at a time, as in addF
Done
--
To view, visit http://gerrit.cloudera.org:8080/12174
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Will Berkeley has posted comments on this change. (
http://gerrit.cloudera.org:8080/12138 )
Change subject: Support location awareness in READ_CLOSEST for the C++ client
..
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@406
PS7, Line 406: random
> nit: does it make sense to add a TODO for the future to make use of the loc
Done
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@408
PS7, Line 408: vector local;
: vector same_location;
> nit: call reserve(filtered.size()) for local and same_location?
Done
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@414
PS7, Line 414: const string replica_location = rts->location();
> nit: maybe, gate computing/calling this based on !client_location.empty()?
Done
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@422
PS7, Line 422: rand
> Do we init random generator in the client library? I guess my question is
After looking at this with Alexey, we think we don't seed rand(), but we should
be. He said he would look at it as a separate changelist.
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/integration-tests/location_assignment-itest.cc
File src/kudu/integration-tests/location_assignment-itest.cc:
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/integration-tests/location_assignment-itest.cc@79
PS7, Line 79: ThreadSafeRandom rng(SeedRandom());
> nit: it seems TabletServerIntegrationTestBase has ThreadSafeRandom as aggre
Done
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/integration-tests/location_assignment-itest.cc@82
PS7, Line 82: auto location = Substitute("/L$0", i);
> Now unused.
Done
http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/integration-tests/location_assignment-itest.cc@125
PS7, Line 125: // When running on Linux, the client and tablet servers each
have their own
: // IP in the local address space, so no tablet server will
be considered
: // local to the client. If there is a tablet server in the
same location as
: // the client, it will be the only tablet server scanned.
Otherwise, some
: // random tablet server will be scanned.
> nit: move this under the #if defined block below?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12138
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713
Gerrit-Change-Number: 12138
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Tue, 08 Jan 2019 18:58:03 +
Gerrit-HasComments: Yes
[kudu-CR] tablet: clean up MergeIterState API
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12176 ) Change subject: tablet: clean up MergeIterState API .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12176/3/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12176/3/src/kudu/common/generic_iterators.cc@126 PS3, Line 126: // Exposed for optimization purposes. I don't understand this part. We use remaining_in_block() to resize the client's RowBlock; isn't that necessary for correctness? -- To view, visit http://gerrit.cloudera.org:8080/12176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f821dc06ddbe3f7ef018eec15b1993cde7e7e0 Gerrit-Change-Number: 12176 Gerrit-PatchSet: 3 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 08 Jan 2019 18:55:16 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12178 ) Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Patch Set 2: Code-Review+1 Nothing to add to Alexey's comments. -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 18:51:04 + Gerrit-HasComments: No
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 ) Change subject: Support location awareness in READ_CLOSEST for the C++ client .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@422 PS7, Line 422: rand > I opened a TODO item to verify that: https://issues.apache.org/jira/browse/ I meant 'Let that be out of the scope for this changelist.' -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 7 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 18:17:23 + Gerrit-HasComments: Yes
[kudu-CR] Support location awareness in READ CLOSEST for the C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12138 ) Change subject: Support location awareness in READ_CLOSEST for the C++ client .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/12138/7/src/kudu/client/client-internal.cc@422 PS7, Line 422: rand > Do we init random generator in the client library? I guess my question is I opened a TODO item to verify that: https://issues.apache.org/jira/browse/KUDU-2657 I think we can about this to be out of the scope for this changelist. -- To view, visit http://gerrit.cloudera.org:8080/12138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2c6bcc7479c5cf2e17cb6e368ca89a1eb7f21713 Gerrit-Change-Number: 12138 Gerrit-PatchSet: 7 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 08 Jan 2019 18:13:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/12178 )
Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder
..
Patch Set 2:
(4 comments)
A few nits
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/delta_tracker.cc@207
PS2, Line 207: Status DeltaTracker::ValidateDeltaOrder(const
std::shared_ptr& first,
: const
std::shared_ptr& second,
: const IOContext*
io_context,
: DeltaType type) {
: shared_ptr first_copy = first;
: shared_ptr second_copy = second;
:
: // Make clones so we don't leave the original ones initted.
That can affect
: // tests. We know it's a DeltaFileReader if it's not Initted().
: if (!first_copy->Initted()) {
: shared_ptr first_clone;
:
RETURN_NOT_OK(down_cast(first.get())->CloneForDebugging(
: rowset_metadata_->fs_manager(),
mem_trackers_.tablet_tracker, _clone));
: RETURN_NOT_OK(first_clone->Init(io_context));
: first_copy = first_clone;
: }
: if (!second_copy->Initted()) {
: shared_ptr second_clone;
:
RETURN_NOT_OK(down_cast(second.get())->CloneForDebugging(
: rowset_metadata_->fs_manager(),
mem_trackers_.tablet_tracker, _clone));
: RETURN_NOT_OK(second_clone->Init(io_context));
: second_copy = second_clone;
: }
:
: switch (type) {
: case REDO:
: DCHECK_LE(first_copy->delta_stats().min_timestamp(),
: second_copy->delta_stats().min_timestamp())
: << "Found out-of-order deltas: [{" <<
first_copy->ToString() << "}, {"
: << second_copy->ToString() << "}]: type = " << type;
: break;
: case UNDO:
: DCHECK_GE(first_copy->delta_stats().min_timestamp(),
: second_copy->delta_stats().min_timestamp())
: << "Found out-of-order deltas: [{" <<
first_copy->ToString() << "}, {"
: << second_copy->ToString() << "}]: type = " << type;
: break;
: }
: return Status::OK();
: }
:
: Status DeltaTracker::ValidateDeltasOrdered(const
SharedDeltaStoreVector& list,
:const IOContext*
io_context,
:DeltaType type) {
: for (size_t i = 1; i < list.size(); i++) {
: RETURN_NOT_OK(ValidateDeltaOrder(list[i - 1], list[i],
io_context, type));
: }
: return Status::OK();
: }
If this is not used elsewhere but only in ifndef NDEBUG case, maybe put those
under '#ifndef NDEBUG' as well?
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc
File src/kudu/tablet/major_delta_compaction-test.cc:
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@186
PS2, Line 186: const
nit: constexpr ?
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@191
PS2, Line 191: all_rowsets.front();
paranoid nit: is it guaranteed that all_rowsets container is non-empty?If
not, maybe add ASSERT_FALSE(all_rowsets.empty())?
http://gerrit.cloudera.org:8080/#/c/12178/2/src/kudu/tablet/major_delta_compaction-test.cc@205
PS2, Line 205: ASSERT_TRUE(s.IsCorruption());
nit: maybe, add ' << s.ToString(); for easier debugging if that fails
--
To view, visit http://gerrit.cloudera.org:8080/12178
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b
Gerrit-Change-Number: 12178
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 18:05:39 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Andrew Wong has removed a vote on this change. Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Removed -Verified by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12178 ) Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Patch Set 2: Verified+1 The failure seems unrelated: RaftConsensusNonVoterITest.PromoteAndDemote -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 16:49:27 + Gerrit-HasComments: No
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Andrew Wong has removed a vote on this change. Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Assign locations to tablet servers and the client in Java
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/12174 )
Change subject: Assign locations to tablet servers and the client in Java
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java@49
PS3, Line 49: --
nit: ' --' or replace with ';'
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java:
PS3:
> What happened to the formatting in this file?
I guess the very first version of this file was had DOS end-of-line symbols,
and it's still partially true.
Will, while you are at at, maybe run dos2unix for this file?
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java@1
PS3, Line 1: /**
: * Licensed under the Apache License, Version 2.0 (the "License");
: * you may not use this file except in compliance with the License.
: * You may obtain a copy of the License at
: *
: * http://www.apache.org/licenses/LICENSE-2.0
: *
: * Unless required by applicable law or agreed to in writing,
software
: * distributed under the License is distributed on an "AS IS"
BASIS,
: * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
: * See the License for the specific language governing permissions
and
: * limitations under the License. See accompanying LICENSE file.
: */
nit: while you are at it, maybe unify the license header and make it commented
by single-line comments?
Also, it seems the text is little bit different from the standard license
header, but I'm not sure whether it's crucial to standardize that as well.
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
File
java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java:
http://gerrit.cloudera.org:8080/#/c/12174/3/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@229
PS3, Line 229: getClass().getResource("/assign-location.py").getFile()
I'm curious whether this works when tests are run using dist-test.
--
To view, visit http://gerrit.cloudera.org:8080/12174
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e2c74ab12f7da187bf6e75d42a3089bc20235db
Gerrit-Change-Number: 12174
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 15:22:21 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2656: pass IOContext to ValidateDeltaOrder
Hello Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12178 to look at the new patch set (#2). Change subject: KUDU-2656: pass IOContext to ValidateDeltaOrder .. KUDU-2656: pass IOContext to ValidateDeltaOrder Previously, checksum errors that occurred during calls to the debug-only function ValidateDeltaOrder() would lead to a DCHECK failure. This patch plumbs an IOContext to these calls to avoid this and adds a regression test for such cases. Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b --- M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 6 files changed, 55 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/12178/2 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2565: pass IOContext to ValidateDeltaOrder
Hello Adar Dembo, Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12178 to review the following change. Change subject: KUDU-2565: pass IOContext to ValidateDeltaOrder .. KUDU-2565: pass IOContext to ValidateDeltaOrder Previously, checksum errors that occurred during calls to the debug-only function ValidateDeltaOrder() would lead to a DCHECK failure. This patch plumbs an IOContext to these calls to avoid this and adds a regression test for such cases. Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b --- M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h 6 files changed, 54 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/12178/1 -- To view, visit http://gerrit.cloudera.org:8080/12178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3364ad95a5b3608db6538151007c4b6d16500f2b Gerrit-Change-Number: 12178 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke
