[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8037/1//COMMIT_MSG Commit Message: PS1, Line 32: causing increased latency That part is less intuitive to me, where's the slowdown coming from? The node you paused happened to be the fastest of the two followers? -- To view, visit http://gerrit.cloudera.org:8080/8037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Update website for 1.5.0 release
Dan Burkert has uploaded a new patch set (#2). Change subject: Update website for 1.5.0 release .. Update website for 1.5.0 release Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a --- M apidocs/allclasses-frame.html M apidocs/allclasses-noframe.html M apidocs/constant-values.html M apidocs/deprecated-list.html M apidocs/help-doc.html M apidocs/index-all.html M apidocs/index.html M apidocs/org/apache/kudu/ColumnSchema.html M apidocs/org/apache/kudu/Schema.html M apidocs/org/apache/kudu/Type.html D apidocs/org/apache/kudu/annotations/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/InterfaceStability.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceStability.html D apidocs/org/apache/kudu/annotations/package-frame.html D apidocs/org/apache/kudu/annotations/package-summary.html D apidocs/org/apache/kudu/annotations/package-tree.html D apidocs/org/apache/kudu/annotations/package-use.html M apidocs/org/apache/kudu/class-use/ColumnSchema.html M apidocs/org/apache/kudu/class-use/Schema.html M apidocs/org/apache/kudu/class-use/Type.html M apidocs/org/apache/kudu/client/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AlterTableOptions.html M apidocs/org/apache/kudu/client/AlterTableResponse.html M apidocs/org/apache/kudu/client/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduClient.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/AsyncKuduSession.html M apidocs/org/apache/kudu/client/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/CreateTableOptions.html M apidocs/org/apache/kudu/client/Delete.html M apidocs/org/apache/kudu/client/DeleteTableResponse.html M apidocs/org/apache/kudu/client/ExternalConsistencyMode.html M apidocs/org/apache/kudu/client/HasFailedRpcException.html M apidocs/org/apache/kudu/client/Insert.html M apidocs/org/apache/kudu/client/IsAlterTableDoneResponse.html M apidocs/org/apache/kudu/client/KuduClient.KuduClientBuilder.html M apidocs/org/apache/kudu/client/KuduClient.html M apidocs/org/apache/kudu/client/KuduException.html M apidocs/org/apache/kudu/client/KuduPredicate.ComparisonOp.html M apidocs/org/apache/kudu/client/KuduPredicate.html M apidocs/org/apache/kudu/client/KuduScanToken.KuduScanTokenBuilder.html M apidocs/org/apache/kudu/client/KuduScanToken.html M apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html M apidocs/org/apache/kudu/client/KuduScanner.html M apidocs/org/apache/kudu/client/KuduSession.html M apidocs/org/apache/kudu/client/KuduTable.html M apidocs/org/apache/kudu/client/ListTablesResponse.html M apidocs/org/apache/kudu/client/ListTabletServersResponse.html M apidocs/org/apache/kudu/client/LocatedTablet.Replica.html M apidocs/org/apache/kudu/client/LocatedTablet.html M apidocs/org/apache/kudu/client/Operation.html M apidocs/org/apache/kudu/client/OperationResponse.html M apidocs/org/apache/kudu/client/PartialRow.html M apidocs/org/apache/kudu/client/PleaseThrottleException.html M apidocs/org/apache/kudu/client/RangePartitionBound.html M apidocs/org/apache/kudu/client/ReplicaSelection.html M apidocs/org/apache/kudu/client/RowError.html M apidocs/org/apache/kudu/client/RowErrorsAndOverflowStatus.html M apidocs/org/apache/kudu/client/RowResult.html M apidocs/org/apache/kudu/client/RowResultIterator.html M apidocs/org/apache/kudu/client/SessionConfiguration.FlushMode.html M apidocs/org/apache/kudu/client/SessionConfiguration.html M apidocs/org/apache/kudu/client/Statistics.Statistic.html M apidocs/org/apache/kudu/client/Statistics.html M apidocs/org/apache/kudu/client/Status.html M apidocs/org/apache/kudu/client/Update.html M apidocs/org/apache/kudu/client/Upsert.html M apidocs/org/apache/kudu/client/class-use/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AlterTableOptions.html M apidocs/org/apache/kudu/client/class-use/AlterTableResponse.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduSession.html M apidocs/org/apache/kudu/client/class-use/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/class-use/CreateTableOptions.html M apidocs/org/apache/kudu/client/class-use/Delete.html M apidocs/org/apache/kudu/client/class-use/DeleteTableResponse.html M apidocs/org/apache/kudu/client/class-use/ExternalConsistencyMode.html M apidocs/o
[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.
David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries. .. Patch Set 1: does this have any nasty interaction with failure detection for other situations than the one described in the jira. It seems like it might potentially have since the previous timeout was less than 3x heartbeats and the new one isn't. -- To view, visit http://gerrit.cloudera.org:8080/8037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Alexey Serbin has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 320: ASSERT_FALSE(HasFailure()); > NO_FATALS, through some unknowable black magic, only triggers when there ar I'm curious why it's necessary to add this assert. What happens if there is a failure at least in one thread above and this ASSERT_FALSE(HasFailure()) is not present? Does the test pass in that case or there is some other misreporting? -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 320: ASSERT_FALSE(HasFailure()); > I'm curious why it's necessary to add this assert. What happens if there i It still fails even without this statement. -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Update website for 1.5.0 release
Dan Burkert has posted comments on this change. Change subject: Update website for 1.5.0 release .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8013/1/releases/index.md File releases/index.md: Line 12: * [Kudu 1.5.0](1.5.0/)** was released on September 8, 2017. > Are you missing a '**' here for proper bold rendering? Done -- To view, visit http://gerrit.cloudera.org:8080/8013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Update website for 1.5.0 release
Todd Lipcon has posted comments on this change. Change subject: Update website for 1.5.0 release .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Update website for 1.5.0 release
Dan Burkert has posted comments on this change. Change subject: Update website for 1.5.0 release .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Update website for 1.5.0 release
Dan Burkert has submitted this change and it was merged. Change subject: Update website for 1.5.0 release .. Update website for 1.5.0 release Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a Reviewed-on: http://gerrit.cloudera.org:8080/8013 Reviewed-by: Todd Lipcon Tested-by: Dan Burkert --- M apidocs/allclasses-frame.html M apidocs/allclasses-noframe.html M apidocs/constant-values.html M apidocs/deprecated-list.html M apidocs/help-doc.html M apidocs/index-all.html M apidocs/index.html M apidocs/org/apache/kudu/ColumnSchema.html M apidocs/org/apache/kudu/Schema.html M apidocs/org/apache/kudu/Type.html D apidocs/org/apache/kudu/annotations/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/InterfaceStability.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceAudience.html D apidocs/org/apache/kudu/annotations/class-use/InterfaceStability.html D apidocs/org/apache/kudu/annotations/package-frame.html D apidocs/org/apache/kudu/annotations/package-summary.html D apidocs/org/apache/kudu/annotations/package-tree.html D apidocs/org/apache/kudu/annotations/package-use.html M apidocs/org/apache/kudu/class-use/ColumnSchema.html M apidocs/org/apache/kudu/class-use/Schema.html M apidocs/org/apache/kudu/class-use/Type.html M apidocs/org/apache/kudu/client/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AlterTableOptions.html M apidocs/org/apache/kudu/client/AlterTableResponse.html M apidocs/org/apache/kudu/client/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduClient.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/AsyncKuduSession.html M apidocs/org/apache/kudu/client/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/CreateTableOptions.html M apidocs/org/apache/kudu/client/Delete.html M apidocs/org/apache/kudu/client/DeleteTableResponse.html M apidocs/org/apache/kudu/client/ExternalConsistencyMode.html M apidocs/org/apache/kudu/client/HasFailedRpcException.html M apidocs/org/apache/kudu/client/Insert.html M apidocs/org/apache/kudu/client/IsAlterTableDoneResponse.html M apidocs/org/apache/kudu/client/KuduClient.KuduClientBuilder.html M apidocs/org/apache/kudu/client/KuduClient.html M apidocs/org/apache/kudu/client/KuduException.html M apidocs/org/apache/kudu/client/KuduPredicate.ComparisonOp.html M apidocs/org/apache/kudu/client/KuduPredicate.html M apidocs/org/apache/kudu/client/KuduScanToken.KuduScanTokenBuilder.html M apidocs/org/apache/kudu/client/KuduScanToken.html M apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html M apidocs/org/apache/kudu/client/KuduScanner.html M apidocs/org/apache/kudu/client/KuduSession.html M apidocs/org/apache/kudu/client/KuduTable.html M apidocs/org/apache/kudu/client/ListTablesResponse.html M apidocs/org/apache/kudu/client/ListTabletServersResponse.html M apidocs/org/apache/kudu/client/LocatedTablet.Replica.html M apidocs/org/apache/kudu/client/LocatedTablet.html M apidocs/org/apache/kudu/client/Operation.html M apidocs/org/apache/kudu/client/OperationResponse.html M apidocs/org/apache/kudu/client/PartialRow.html M apidocs/org/apache/kudu/client/PleaseThrottleException.html M apidocs/org/apache/kudu/client/RangePartitionBound.html M apidocs/org/apache/kudu/client/ReplicaSelection.html M apidocs/org/apache/kudu/client/RowError.html M apidocs/org/apache/kudu/client/RowErrorsAndOverflowStatus.html M apidocs/org/apache/kudu/client/RowResult.html M apidocs/org/apache/kudu/client/RowResultIterator.html M apidocs/org/apache/kudu/client/SessionConfiguration.FlushMode.html M apidocs/org/apache/kudu/client/SessionConfiguration.html M apidocs/org/apache/kudu/client/Statistics.Statistic.html M apidocs/org/apache/kudu/client/Statistics.html M apidocs/org/apache/kudu/client/Status.html M apidocs/org/apache/kudu/client/Update.html M apidocs/org/apache/kudu/client/Upsert.html M apidocs/org/apache/kudu/client/class-use/AbstractKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AlterTableOptions.html M apidocs/org/apache/kudu/client/class-use/AlterTableResponse.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.ReadMode.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.html M apidocs/org/apache/kudu/client/class-use/AsyncKuduSession.html M apidocs/org/apache/kudu/client/class-use/ColumnRangePredicate.html M apidocs/org/apache/kudu/client/class-use/CreateTableOptions.html M apidocs/org/apache/kudu/client/class-use/Delete.html M apidocs/org/apache/kudu/client/class-u
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Alexey Serbin has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 8: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 320: ASSERT_FALSE(HasFailure()); > It still fails even without this statement. I see, thanks. Maybe, remove it then (or the idea is to catch some other errors)? -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] docs: clarify steps for removing master from multi-master deployment
Alexey Serbin has posted comments on this change. Change subject: docs: clarify steps for removing master from multi-master deployment .. Patch Set 2: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/8032/1/docs/administration.adoc File docs/administration.adoc: PS1, Line 376: has > I think "been" is fine here; I'm using it here as a verb, as in: SGTM http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc File docs/administration.adoc: PS2, Line 392: masters nit: master nodes? -- To view, visit http://gerrit.cloudera.org:8080/8032 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Dan Burkert has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: Line 320: ASSERT_FALSE(HasFailure()); > I see, thanks. Maybe, remove it then (or the idea is to catch some other e Done -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8016 to look at the new patch set (#10). Change subject: KUDU-2125: Tablet copy client does not retry on failures .. KUDU-2125: Tablet copy client does not retry on failures The tablet copy client would fail the tablet copy after encountering an RPC error. This commit adds retry logic for tablet copy operations when encountering SERVER_TOO_BUSY or UNAVAILABLE errors, which are retriable. Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy_client_session-itest.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_client.h 5 files changed, 133 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8016/10 -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Adar Dembo has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 10: Code-Review+1 Mike should review this. -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 8: Code-Review+1 Mike should review this. -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] docs: clarify steps for removing master from multi-master deployment
Adar Dembo has posted comments on this change. Change subject: docs: clarify steps for removing master from multi-master deployment .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/8032/1/docs/administration.adoc File docs/administration.adoc: Line 374: Removing masters from a Multi-Master Deployment > Done. I don't know; I think you should ask Mike for clarification. I don't think ksck can tell you which master is most 'up-to-date'. http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc File docs/administration.adoc: Line 379: WARNING: in planning the new multi-master configuration, keep in mind that the number of masters Nit: other WARNING text begins with a capital letter. Below too. PS2, Line 382: WARNING: dropping the number of masters below the number of masters currently needed for a Raft : majority can incur data loss. Please double check this with Mike. PS2, Line 392: masters > nit: master nodes? I don't really understand why this instruction is worth including. Yes, it makes sense to ensure that the masters that you're removing aren't going to come back. But I think that's sort of implicit in these instructions, especially in the "Remove the directories..." step. If you look at the other master workflows, none of them call this out explicitly, so at the very least, this workflow ought to be consistent with them. -- To view, visit http://gerrit.cloudera.org:8080/8032 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Kudu Consistency Blog Post Pt1
Will Berkeley has posted comments on this change. Change subject: Kudu Consistency Blog Post Pt1 .. Patch Set 12: (3 comments) Just a couple of things + Andrew's nits http://gerrit.cloudera.org:8080/#/c/7019/12/_posts/2017-08-21-kudu-consistency-pt1.md File _posts/2017-08-21-kudu-consistency-pt1.md: PS12, Line 35: prevent > nit: consider "that prevent" +1 PS12, Line 57: availability This is a touch confusing because availability is being maximized by minimizing insertion errors + insert latency. PS12, Line 60: , Remove -- To view, visit http://gerrit.cloudera.org:8080/7019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icaec0c8ace20651a65901a8e1786e785265540d1 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2055 [part 2]: Add util to construct sorted disjoint interval
Hao Hao has uploaded a new change for review. http://gerrit.cloudera.org:8080/8041 Change subject: KUDU-2055 [part 2]: Add util to construct sorted disjoint interval .. KUDU-2055 [part 2]: Add util to construct sorted disjoint interval This patch adds an utility to construct a sorted disjoint interval set given a set of intervals. The operation to construct such one is O(nlgn + n) where 'n' is the number of intervals. For example, given the input interval set: |--1---| |-2-| |--3--||---4--||5| The output sorted disjoint interval set is: |--1--| |-2-| It also adds unit test to verify given overlap, duplicate, invalid intervals, the implementation works as expected. Change-Id: I61a813c047be4882f246eaf404598e7e18fcac87 --- M src/kudu/util/CMakeLists.txt A src/kudu/util/sorted_disjoint_interval-test.cc A src/kudu/util/sorted_disjoint_interval.h 3 files changed, 261 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8041/1 -- To view, visit http://gerrit.cloudera.org:8080/8041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I61a813c047be4882f246eaf404598e7e18fcac87 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao
[kudu-CR] KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients
Hello Dan Burkert, Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8026 to look at the new patch set (#2). Change subject: KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients .. KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients This patch modifies new Java clients to no longer use the create_table_done field in GetTableSchema RPCs. After the CreateTable RPC returns, the client will enter an IsCreateTableDone loop, and when that's done, it'll issue a GetTableSchema to fetch the schema and build a KuduTable. There are a couple other changes worth noting: - openTable no longer waits for table creation before returning. Doing that now would require at least one IsCreateTableDone RPC, which basically defeats the purpose of these changes. Moreover, the C++ client doesn't do this, so I don't see why the Java client should. - I removed the 'tablesNotServed' logic which had no useful effect because, as I recently learned, there's retry logic to deal with TABLET_NOT_RUNNING master errors embedded more deeply in the Java client. - I removed the master permit acquisition from the "is create table done" loop, since these loops should no longer produce a thundering herds (as I suppose they could have due to 'tablesNotServed'). - Even though createTable waits for table creation to finish, I added an isCreateTableDone method to help synchronize the use case where multiple clients or threads all try to create and then open a table concurrently. Change-Id: I54fa07dc34a97f1c9da06ec9129d6d4590b7aac6 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java M java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaRequest.java M java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java A java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestDeadlineTracker.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestFlexiblePartitioning.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java 12 files changed, 398 insertions(+), 253 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/8026/2 -- To view, visit http://gerrit.cloudera.org:8080/8026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I54fa07dc34a97f1c9da06ec9129d6d4590b7aac6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients
Adar Dembo has posted comments on this change. Change subject: KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients .. Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/8026/1//COMMIT_MSG Commit Message: PS1, Line 23: Meanwhile > I read this to mean that this patch is both modifying the master as describ I'll try to clarify while still providing context to understand the motivation for this patch. http://gerrit.cloudera.org:8080/#/c/8026/1/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: Line 524:* Gets a table's schema > Missing period Done http://gerrit.cloudera.org:8080/#/c/8026/1/java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java File java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java: Line 36: IsCreateTableDoneRequest(KuduTable table, String name) { > Doesn't identifying the table by name open the client up to a race conditio I suppose so, but IsAlterTableDoneRequest behaves this way, as do both IsAlterTableDone and IsCreateTableDone on the C++ side. Moreover, in the standalone case (i.e. if a user just makes those client calls), the table name is all you have anyway. So I'd prefer to keep them consistent. http://gerrit.cloudera.org:8080/#/c/8026/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java: Line 99: public boolean isCreateTableDone(String name) throws KuduException { > Why is this not a joinAndHandleException wrapper around an async variant? I was copying isAlterTableDone without thinking about it too hard. In order to actually wait for the creation to finish, joining the deferred is insufficient; we need a loop that sleeps and retries too. Line 101: while (totalSleepTime < getDefaultAdminOperationTimeoutMs()) { > This implementation appears to be waiting for the table to be created if it I don't want to change the method name (again, to be consistent with IsAlterTableDone), but I will reword the Javadocs to make this behavior more clear. Line 121: LOG.debug("Create not done, sleep " + (AsyncKuduClient.SLEEP_TIME - elapsed) + > This log message doesn't make a lot of sense in isolation, and restructure I think it's OK that it doesn't make much sense since it's debug-level (something we'd normally enable just for isolated testing). I will restructure it (and the version in IsAlterTableDone) though. Line 132: return false; > Shouldn't this throw a timeout exception? That's not the behavior of IsAlterTableDone, and I was trying to be consistent with it. http://gerrit.cloudera.org:8080/#/c/8026/1/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: Line 940: if (e.getMessage().contains("does not exist")) { > I think this could be more precise by asserting on the returned Status (Not Done Line 954: Insert insert = table.newInsert(); > Shouldn't this work even without waiting for isCreateTableDone? I would ha Yes, we discussed this yesterday. Will make the change. -- To view, visit http://gerrit.cloudera.org:8080/8026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54fa07dc34a97f1c9da06ec9129d6d4590b7aac6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-1807 (part 3): remove GetTableSchema.create table done
Hello Dan Burkert, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8045 to review the following change. Change subject: KUDU-1807 (part 3): remove GetTableSchema.create_table_done .. KUDU-1807 (part 3): remove GetTableSchema.create_table_done Since this field was optional, and since the Java client is no longer using it, let's remove it entirely. The protobuf 'reserved field' functionality ensures that the field number isn't accidentally reused in the future. Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830 --- M src/kudu/master/master.proto 1 file changed, 3 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/8045/1 -- To view, visit http://gerrit.cloudera.org:8080/8045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1807 (part 1): deprecate GetTableSchema.create table done
Hello Dan Burkert, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8027 to look at the new patch set (#2). Change subject: KUDU-1807 (part 1): deprecate GetTableSchema.create_table_done .. KUDU-1807 (part 1): deprecate GetTableSchema.create_table_done Improving IsCreateInProgress performance in the master is very challenging due to the myriad of locks in play. A simpler alternative would be to get it off any critical paths. It's currently called by the IsCreateTableDone and GetTableSchema RPCs. The former makes sense but the latter does not: if we need to know whether a table has been created, well, that's what the IsCreateTableDone RPC is for. So, does anyone use the "is table done creating" aspect of GetTableSchema? The C++ client doesn't, but the Java client does. As luck would have it, the Java client is already written to fall back to IsCreateTableDone if GetTableSchema says the table is still being created, which means we can safely modify the master to always return false in this field without breaking old Java clients. Future patches will change the behavior of the Java client and remove the create_table_done field altogether. For now, the existence of this deprecation patch helps to ensure that the Java client does, in fact, work properly if the field's value is always false. Change-Id: Ia33bbb2abaabb97db1613d442a7d065710048cc2 --- M src/kudu/integration-tests/registration-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto 3 files changed, 3 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/8027/2 -- To view, visit http://gerrit.cloudera.org:8080/8027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia33bbb2abaabb97db1613d442a7d065710048cc2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1807 (part 3): remove GetTableSchema.create table done
Adar Dembo has posted comments on this change. Change subject: KUDU-1807 (part 3): remove GetTableSchema.create_table_done .. Patch Set 1: Verified+1 NTP synchronization error in one of the dist-test slaves, apparently. -- To view, visit http://gerrit.cloudera.org:8080/8045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.
Mike Percy has posted comments on this change. Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8037/1/src/kudu/consensus/consensus_peers.cc File src/kudu/consensus/consensus_peers.cc: Line 51: DEFINE_int32(consensus_rpc_timeout_ms, 3, This doesn't appear to prevent stacking, does it? I don't see a mechanism for the heartbeater not to trigger Peer::SignalRequest() while a consensus request is outstanding. Is that what we want? -- To view, visit http://gerrit.cloudera.org:8080/8037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add tablet state summary metrics
Mike Percy has posted comments on this change. Change subject: Add tablet state summary metrics .. Patch Set 7: (1 comment) looks good, just one nit on something that changed since the last review http://gerrit.cloudera.org:8080/#/c/7980/7/src/kudu/tserver/ts_tablet_manager_metrics.h File src/kudu/tserver/ts_tablet_manager_metrics.h: Line 47: int num_not_initialized_; nit: don't suffix public struct fields with _ per https://google.github.io/styleguide/cppguide.html#Variable_Names -- To view, visit http://gerrit.cloudera.org:8080/7980 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Mike Percy has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 8: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/7985/1/src/kudu/tserver/tablet_copy_source_session.cc File src/kudu/tserver/tablet_copy_source_session.cc: Line 166: if (!reader) { > Hmm, I'm not seeing where the log_ field in TabletReplica is getting reset? Ah, you're right. http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/tserver/tablet_copy_source_session.cc File src/kudu/tserver/tablet_copy_source_session.cc: Line 73: TAG_FLAG(tablet_copy_session_inject_latency_on_init_ms, hidden); > Done Unsafe implies hidden, so explicit hidden isn't actually needed. But not a big deal. -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Mike Percy has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 8: -Code-Review (1 comment) http://gerrit.cloudera.org:8080/#/c/7985/8/src/kudu/integration-tests/cluster_itest_util.cc File src/kudu/integration-tests/cluster_itest_util.cc: Line 1019: *error_code = error.code(); I'm not sure if you did this or I did this, but if error_code is passed in as nullptr this will result in a segfault, which is bad because we default it to nullptr in the header file. -- To view, visit http://gerrit.cloudera.org:8080/7985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-2135 (part 1): add persistent disk states
Andrew Wong has uploaded a new change for review. http://gerrit.cloudera.org:8080/8048 Change subject: KUDU-2135 (part 1): add persistent disk states .. KUDU-2135 (part 1): add persistent disk states This patch introduces a new format for PathInstanceMetadataFiles that includes a disk health and path for each data directory, and a timestamp that the path instance file was last updated. These additions will be necessary to ensure failed disks will not be used and that data is where expect it to be (which will be increasingly important as we begin to supporting removal/addition/replacement of disks). The expected usage of the new format would enforce startup restrictions on the server (e.g. to not use a failed disk, to not use data if it is on a different disk than expected, etc.) and tooling does not yet exist to work around these restrictions. As such, this patch will upgrade and maintain the new path sets as necessary, persisting disk health when disks fail, but will not apply the mentioned restrictions at startup. A test is added to data_dirs-test to ensure the path instance files get updated/upgraded appropriately. Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs.proto 7 files changed, 467 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/8048/1 -- To view, visit http://gerrit.cloudera.org:8080/8048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong
[kudu-CR](gh-pages) Kudu Consistency Blog Post Pt1
Alexey Serbin has posted comments on this change. Change subject: Kudu Consistency Blog Post Pt1 .. Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/7019/12/_posts/2017-08-21-kudu-consistency-pt1.md File _posts/2017-08-21-kudu-consistency-pt1.md: PS12, Line 54: > nit: Dynamo[3]-inspired? Dynamo-inspired[3]? another option might be: ... systems inspired by _Dynamo_[3], ... PS12, Line 104: so Consider starting a new sentence from here for better readability: ... feature). So, when discussing ... PS12, Line 119: wall time wall clock time ? PS12, Line 152: programatically typo: programmatically Line 171: [[3]](www.allthingsdistributed.com/files/amazon-dynamo-sosp2007.pdf): Giuseppe DeCandia, Deniz Hastorun, Madan Jampani, Gunavardhan Kakulapati, Avinash Lakshman, Alex Pilchin, Swaminathan Sivasubramanian, Peter Vosshall, and Werner Vogels. 2007. Dynamo: amazon's highly available key-value store. In Proceedings of twenty-first ACM SIGOPS symposium on Operating systems principles (SOSP '07). ACM, New York, NY, USA nit: consider adding the protocol prefix 'http://' -- To view, visit http://gerrit.cloudera.org:8080/7019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icaec0c8ace20651a65901a8e1786e785265540d1 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures
Mike Percy has posted comments on this change. Change subject: KUDU-2125: Tablet copy client does not retry on failures .. Patch Set 10: (5 comments) Just one thing I'm wondering about, in the tablet copy client. http://gerrit.cloudera.org:8080/#/c/8016/10/src/kudu/integration-tests/cluster_itest_util.cc File src/kudu/integration-tests/cluster_itest_util.cc: PS10, Line 856: std::remove_if fancy PS10, Line 858: != Neat: I didn't know you could check for equality between a thing and an optional of a thing. http://gerrit.cloudera.org:8080/#/c/8016/4/src/kudu/integration-tests/tablet_copy_client_session-itest.cc File src/kudu/integration-tests/tablet_copy_client_session-itest.cc: PS4, Line 305: or Turns out it was caught because of some existing calls to HasFailure() in e I'm surprised at this. I've had weird issues trying to do this before and now I always use CHECK_OK outside of the main thread. Since the docs claim otherwise, do you have any evidence that this is safe to do? http://gerrit.cloudera.org:8080/#/c/8016/10/src/kudu/tserver/tablet_copy_client.cc File src/kudu/tserver/tablet_copy_client.cc: PS10, Line 702: >= why >= and not == ? Line 747: // Polynomial backoff with 50% jitter. Have you used this before? I plotted y=10x^2 and it seems pretty good -- well behaved and no need for a cap. -- To view, visit http://gerrit.cloudera.org:8080/8016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] docs: clarify steps for removing master from multi-master deployment
Mike Percy has posted comments on this change. Change subject: docs: clarify steps for removing master from multi-master deployment .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc File docs/administration.adoc: PS2, Line 382: WARNING: dropping the number of masters below the number of masters currently needed for a Raft : majority can incur data loss. > Please double check this with Mike. You are technically correct (the best kind of correct) but there are nuances we might consider explaining here. Data loss is kind of a funny concept in this case because we're really talking about *metadata* loss, which can actually be catastrophic: for example it can cause the master to end up dropping whole tables, I think, depending on how far behind the remaining node was, like if it went offline before you created a new table or partition. However, I'm not sure whether we've ever tested this scenario and I'm pretty confident that we don't have an *automated* test for it either. This is particularly scary if you had one master that was down or partitioned for a long time, and you end up removing the other two, and then this stale remaining master comes back online and is all we have left. In general, if you remove more than a majority at once and then do some kind of manual repair, we don't give you any durability guarantees at all. -- To view, visit http://gerrit.cloudera.org:8080/8032 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-2135 (part 1): add persistent disk states
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8048 to look at the new patch set (#2). Change subject: KUDU-2135 (part 1): add persistent disk states .. KUDU-2135 (part 1): add persistent disk states This patch introduces a new format for PathInstanceMetadataFiles that includes a disk health and path for each data directory, and a timestamp that the path instance file was last updated. These additions will be necessary to ensure failed disks will not be used and that data is where expect it to be (which will be increasingly important as we begin to supporting removal/addition/replacement of disks). The expected usage of the new format would enforce startup restrictions on the server (e.g. to not use a failed disk, to not use data if it is on a different disk than expected, etc.) and tooling does not yet exist to work around these restrictions. As such, this patch will upgrade and maintain the new path sets as necessary, persisting disk health when disks fail, but will not apply the mentioned restrictions at startup. A test is added to data_dirs-test to ensure the path instance files get updated/upgraded appropriately. Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs.proto 7 files changed, 470 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/8048/2 -- To view, visit http://gerrit.cloudera.org:8080/8048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2135 (part 2): check integrity of new on-disk format
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7270 to look at the new patch set (#28). Change subject: KUDU-2135 (part 2): check integrity of new on-disk format .. KUDU-2135 (part 2): check integrity of new on-disk format CheckIntegrity() is updated to ensure that disks that have previously failed are not used and that servers will only open data where it's previously known to be. Rather than comparing all instances against an agreed-upon set of UUIDs, the single path set with the largest timestamp is used as the main one to compare against (if only old path sets are available, the first healthy one is used). If no instances are healthy, CheckIntegrity() will fail, as all disks are failed. Unit testing is done for the new iteration of CheckIntegrity(). Some notes: - Disk failures during FS layout creation are not tolerated. In these cases, there is presumably no data on the server anyway, so operators should easily be able to fix their cluster. - If there are any unhealthy instances when upgrading the path sets (i.e. adding disk states, paths, timestamp), a complete mapping of UUIDs to paths will not be available, and CheckIntegrity() will fail. - The main path set's disk states are updated to reflect the failure of the instances. Since data on a failed disk is not used, disks that are successfully read from but are already marked FAILED by the main path set are not marked HEALTHY. - In the case of a server restart where all healthy disks fail to start up and some known failed disks start working again, the server will successfully start up with the bad disks This patch is a part of a series of patches to handle disk failures. To see how this fits, see 2.6 in: https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing Change-Id: Ifddf0817fe1a82044077f5544c400c88de20769f --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/fs.proto 5 files changed, 328 insertions(+), 117 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/7270/28 -- To view, visit http://gerrit.cloudera.org:8080/7270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifddf0817fe1a82044077f5544c400c88de20769f Gerrit-PatchSet: 28 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add tablet state summary metrics
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7980 to look at the new patch set (#8). Change subject: Add tablet state summary metrics .. Add tablet state summary metrics This patch adds metrics for the number of tablets in each state (i.e. the number of tablets for each value of TabletStatePB). To avoid contention in the tablet manager, the frequency with which the state numbers are computed is limited. Requests that come too soon for an update will receive cached counts. The maximum frequency of updates is controlled by a new flag. Change-Id: I9fda2061d341586f0aa343787af59984a627080a --- M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h A src/kudu/tserver/ts_tablet_manager_metrics.cc A src/kudu/tserver/ts_tablet_manager_metrics.h M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 8 files changed, 379 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/7980/8 -- To view, visit http://gerrit.cloudera.org:8080/7980 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] Add tablet state summary metrics
Will Berkeley has posted comments on this change. Change subject: Add tablet state summary metrics .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/7980/7/src/kudu/tserver/ts_tablet_manager_metrics.h File src/kudu/tserver/ts_tablet_manager_metrics.h: Line 47: int num_not_initialized_; > nit: don't suffix public struct fields with _ per https://google.github.io/ Done -- To view, visit http://gerrit.cloudera.org:8080/7980 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-2135 (part 1): add persistent disk states
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8048 to look at the new patch set (#3). Change subject: KUDU-2135 (part 1): add persistent disk states .. KUDU-2135 (part 1): add persistent disk states This patch introduces a new format for PathInstanceMetadataFiles that includes a health state and path for each data directory, and a timestamp that the path instance file was last updated. These additions will be necessary to ensure that failed disks will not be used and that data is where it is expected be (which will be increasingly important as we begin to supporting removal, addition, and replacement of disks). The expected usage of the new format would enforce startup restrictions on the server (e.g. to never use a previously failed disk, to not use data if it is on a different disk than expected, etc.) and tooling does not yet exist to work around these restrictions. As such, this patch will upgrade and maintain the new path sets as necessary, persisting disk health when disks fail, but will not apply the mentioned restrictions at startup. Tests are added to data_dirs-test to ensure the path instance files get updated and upgraded appropriately. Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 --- M src/kudu/fs/block_manager_util-test.cc M src/kudu/fs/block_manager_util.cc M src/kudu/fs/block_manager_util.h M src/kudu/fs/data_dirs-test.cc M src/kudu/fs/data_dirs.cc M src/kudu/fs/data_dirs.h M src/kudu/fs/fs.proto 7 files changed, 473 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/8048/3 -- To view, visit http://gerrit.cloudera.org:8080/8048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add tablet state summary metrics
Mike Percy has posted comments on this change. Change subject: Add tablet state summary metrics .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/7980/8/src/kudu/tserver/ts_tablet_manager_metrics.h File src/kudu/tserver/ts_tablet_manager_metrics.h: PS8, Line 56: TSTabletManager* ts_tablet_manager; : : FunctionGaugeDetacher metric_detacher; Don't shoot me, but shouldn't these two members be private? Aren't they never meant to be accessed by other classes? -- To view, visit http://gerrit.cloudera.org:8080/7980 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes