[kudu-CR] auto rebalancer: ignore deleted tables
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/15970 ) Change subject: auto_rebalancer: ignore deleted tables .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15970/2/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/15970/2/src/kudu/master/auto_rebalancer-test.cc@679 PS2, Line 679: down nit: downed? -- To view, visit http://gerrit.cloudera.org:8080/15970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5499b09c677dd6c1016349398952a1c39820691 Gerrit-Change-Number: 15970 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 20 May 2020 23:25:35 + Gerrit-HasComments: Yes
[kudu-CR] [k8s] Implement an operator for a KuduCluster CRD
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17566 Change subject: [k8s] Implement an operator for a KuduCluster CRD .. [k8s] Implement an operator for a KuduCluster CRD The operator is a custom controller that monitors and maintains the correct Kudu Kubernetes cluster state (a StatefulSet of Kudu masters, a StatefulSet of Kudu tservers, and Services for the masters, tservers, the master UI). It can be launched as Go program outside the cluster (best for local development) or as a Kubernetes Deployment inside the cluster. Currently, the operator maintains the specified number of masters and tservers, and triggers the Kudu rebalancer if tservers are added or removed. My notes on Kubernetes and Operators here: https://docs.google.com/document/d/1zre8LLq6C0Dh64o16rNknBfp6wwtpytRySXSzUt6prk Change-Id: I1c855f3bab7bbd501f6237a23781f7ae200ba3db --- A kubernetes/kudu-operator/.dockerignore A kubernetes/kudu-operator/.gitignore A kubernetes/kudu-operator/Dockerfile A kubernetes/kudu-operator/Makefile A kubernetes/kudu-operator/PROJECT A kubernetes/kudu-operator/README.adoc A kubernetes/kudu-operator/api/v1/groupversion_info.go A kubernetes/kudu-operator/api/v1/kuducluster_types.go A kubernetes/kudu-operator/api/v1/zz_generated.deepcopy.go A kubernetes/kudu-operator/config/crd/bases/kuduoperator.capstone_kuduclusters.yaml A kubernetes/kudu-operator/config/crd/kustomization.yaml A kubernetes/kudu-operator/config/crd/kustomizeconfig.yaml A kubernetes/kudu-operator/config/crd/patches/cainjection_in_kuduclusters.yaml A kubernetes/kudu-operator/config/crd/patches/webhook_in_kuduclusters.yaml A kubernetes/kudu-operator/config/default/kustomization.yaml A kubernetes/kudu-operator/config/default/manager_auth_proxy_patch.yaml A kubernetes/kudu-operator/config/default/manager_config_patch.yaml A kubernetes/kudu-operator/config/manager/controller_manager_config.yaml A kubernetes/kudu-operator/config/manager/kustomization.yaml A kubernetes/kudu-operator/config/manager/manager.yaml A kubernetes/kudu-operator/config/manifests/kustomization.yaml A kubernetes/kudu-operator/config/prometheus/kustomization.yaml A kubernetes/kudu-operator/config/prometheus/monitor.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_client_clusterrole.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_role.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_role_binding.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_service.yaml A kubernetes/kudu-operator/config/rbac/kuducluster_editor_role.yaml A kubernetes/kudu-operator/config/rbac/kuducluster_viewer_role.yaml A kubernetes/kudu-operator/config/rbac/kustomization.yaml A kubernetes/kudu-operator/config/rbac/leader_election_role.yaml A kubernetes/kudu-operator/config/rbac/leader_election_role_binding.yaml A kubernetes/kudu-operator/config/rbac/role.yaml A kubernetes/kudu-operator/config/rbac/role_binding.yaml A kubernetes/kudu-operator/config/rbac/service_account.yaml A kubernetes/kudu-operator/config/samples/kudu_v1_kuducluster.yaml A kubernetes/kudu-operator/config/samples/kustomization.yaml A kubernetes/kudu-operator/config/scorecard/bases/config.yaml A kubernetes/kudu-operator/config/scorecard/kustomization.yaml A kubernetes/kudu-operator/config/scorecard/patches/basic.config.yaml A kubernetes/kudu-operator/config/scorecard/patches/olm.config.yaml A kubernetes/kudu-operator/controllers/kuducluster_controller.go A kubernetes/kudu-operator/controllers/suite_test.go A kubernetes/kudu-operator/go.mod A kubernetes/kudu-operator/go.sum A kubernetes/kudu-operator/hack/boilerplate.go.txt A kubernetes/kudu-operator/main.go 47 files changed, 3,254 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/17566/1 -- To view, visit http://gerrit.cloudera.org:8080/17566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1c855f3bab7bbd501f6237a23781f7ae200ba3db Gerrit-Change-Number: 17566 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen
[kudu-CR] [k8s] Implement an operator for a KuduCluster CRD
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17566 ) Change subject: [k8s] Implement an operator for a KuduCluster CRD .. Patch Set 1: (27 comments) http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/Makefile File kubernetes/kudu-operator/Makefile: PS1: > nit: could you add a license header to this file? Same for other files that Done http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/Makefile@6 PS1, Line 6: 0.0.7 > Is this supposed to be the Kudu version? Or a different project? This should be the version of the operator; I was just incrementing as I developed/tested. Is there a convention for versioning? http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/Makefile@90 PS1, Line 90: > nit: I know gofmt converts things to tabs, but does it also run on makefile I thought tabs were necessary for Makefile recipes? https://www.gnu.org/software/make/manual/make.html http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/crd/bases/kuduoperator.capstone_kuduclusters.yaml File kubernetes/kudu-operator/config/crd/bases/kuduoperator.capstone_kuduclusters.yaml: PS1: > I expected somewhere in this patch there to be a place where we define what I believe that should be taken care of by the Docker setup. http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/crd/bases/kuduoperator.capstone_kuduclusters.yaml@11 PS1, Line 11: capstone > nit: maybe name this something more generic, like "cluster" or something? S Done http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml File kubernetes/kudu-operator/config/default/kustomization.yaml: http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml@6 PS1, Line 6: alices > nit: kudu-operator-wordpress? Done http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml@19 PS1, Line 19: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in : # crd/kustomization.yaml : #- ../webhook > Seems like this isn't in this patch. Can we scrub away all the "webhook" bi Scrubbing the webhook and certmanager stuff. The prometheus monitor is currently unused when it's commented out; should i just remove that subdirectory entirely? http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml@32 PS1, Line 32: : # Mount the controller config file for loading manager configurations : # through a ComponentConfig type : #- manager_config_patch.yaml > When should users uncomment this line? If you want to modify the configs for the Deployment that runs the operator, you can modify the manager_config_patch.yaml file and uncomment this line: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.7.0/#add-the-manager-config-patch-to-configdefaultkustomizationyaml http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml@37 PS1, Line 37: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in : # crd/kustomization.yaml : #- manager_webhook_patch.yaml : : # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. : # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. : # 'CERTMANAGER' needs to be enabled to use ca injection : #- webhookcainjection_patch.yaml > Same here? Done http://gerrit.cloudera.org:8080/#/c/17566/1/kubernetes/kudu-operator/config/default/kustomization.yaml@45 PS1, Line 45: : # the following config is for teaching kustomize how to do var substitution : vars: : # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. : #- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR : # objref: : #kind: Certificate : #group: cert-manager.io : #version: v1 : #name: serving-cert # this name should match the one in certificate.yaml : # fieldref: : #fieldpath: metadata.namespace : #- name: CERTIFICATE_NAME : # objref: : #kind: Certificate : #group: cert-manager.io : #version: v1 : #name: serving-cert # this name should match the one in certificate.yaml : #- name: SERVICE_NAMESPACE # namespace of the service : # objref: : #kind: Service : #version: v1 : #name: webhook-servic
[kudu-CR] [k8s] Implement an operator for a KuduCluster CRD
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17566 to look at the new patch set (#2). Change subject: [k8s] Implement an operator for a KuduCluster CRD .. [k8s] Implement an operator for a KuduCluster CRD The operator is a custom controller that monitors and maintains the correct Kudu Kubernetes cluster state (a StatefulSet of Kudu masters, a StatefulSet of Kudu tservers, and Services for the masters, tservers, the master UI). It can be launched as Go program outside the cluster (best for local development) or as a Kubernetes Deployment inside the cluster. Currently, the operator maintains the specified number of masters and tservers, and triggers the Kudu rebalancer if tservers are added or removed. My notes on Kubernetes and Operators here: https://docs.google.com/document/d/1zre8LLq6C0Dh64o16rNknBfp6wwtpytRySXSzUt6prk Change-Id: I1c855f3bab7bbd501f6237a23781f7ae200ba3db --- A kubernetes/kudu-operator/.dockerignore A kubernetes/kudu-operator/.gitignore A kubernetes/kudu-operator/Dockerfile A kubernetes/kudu-operator/Makefile A kubernetes/kudu-operator/PROJECT A kubernetes/kudu-operator/README.adoc A kubernetes/kudu-operator/api/v1/groupversion_info.go A kubernetes/kudu-operator/api/v1/kuducluster_types.go A kubernetes/kudu-operator/api/v1/zz_generated.deepcopy.go A kubernetes/kudu-operator/config/crd/bases/kuduoperator.cluster_kuduclusters.yaml A kubernetes/kudu-operator/config/crd/kustomization.yaml A kubernetes/kudu-operator/config/crd/kustomizeconfig.yaml A kubernetes/kudu-operator/config/default/kustomization.yaml A kubernetes/kudu-operator/config/default/manager_auth_proxy_patch.yaml A kubernetes/kudu-operator/config/default/manager_config_patch.yaml A kubernetes/kudu-operator/config/manager/controller_manager_config.yaml A kubernetes/kudu-operator/config/manager/kustomization.yaml A kubernetes/kudu-operator/config/manager/manager.yaml A kubernetes/kudu-operator/config/manifests/kustomization.yaml A kubernetes/kudu-operator/config/prometheus/kustomization.yaml A kubernetes/kudu-operator/config/prometheus/monitor.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_client_clusterrole.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_role.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_role_binding.yaml A kubernetes/kudu-operator/config/rbac/auth_proxy_service.yaml A kubernetes/kudu-operator/config/rbac/kuducluster_editor_role.yaml A kubernetes/kudu-operator/config/rbac/kuducluster_viewer_role.yaml A kubernetes/kudu-operator/config/rbac/kustomization.yaml A kubernetes/kudu-operator/config/rbac/leader_election_role.yaml A kubernetes/kudu-operator/config/rbac/leader_election_role_binding.yaml A kubernetes/kudu-operator/config/rbac/role.yaml A kubernetes/kudu-operator/config/rbac/role_binding.yaml A kubernetes/kudu-operator/config/rbac/service_account.yaml A kubernetes/kudu-operator/config/samples/kudu_v1_kuducluster.yaml A kubernetes/kudu-operator/config/samples/kustomization.yaml A kubernetes/kudu-operator/config/scorecard/bases/config.yaml A kubernetes/kudu-operator/config/scorecard/kustomization.yaml A kubernetes/kudu-operator/config/scorecard/patches/basic.config.yaml A kubernetes/kudu-operator/config/scorecard/patches/olm.config.yaml A kubernetes/kudu-operator/controllers/kuducluster_controller.go A kubernetes/kudu-operator/controllers/suite_test.go A kubernetes/kudu-operator/go.mod A kubernetes/kudu-operator/go.sum A kubernetes/kudu-operator/hack/boilerplate.go.txt A kubernetes/kudu-operator/main.go 45 files changed, 3,744 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/17566/2 -- To view, visit http://gerrit.cloudera.org:8080/17566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1c855f3bab7bbd501f6237a23781f7ae200ba3db Gerrit-Change-Number: 17566 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#5). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 859 insertions(+), 658 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/5 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 5 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#6). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 857 insertions(+), 661 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/6 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 6 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@301
PS6, Line 301: ClusterServerHealth
> Can we forward declare these?
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@613
PS6, Line 613: int table_num_replicas);
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc@87
PS6, Line 87: const map& label_by_uuid,
> nit: keep these on the same line
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h
File src/kudu/tools/placement_policy_util.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h@30
PS6, Line 30: namespace rebalance {
: struct ClusterLocalityInfo;
: } // namespace rebalance
> You can put this in the below kudu namespace.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 21:40:09 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#7). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,884 insertions(+), 1,602 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/7 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: Create kudu/rebalance subdirectory .. Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc File src/kudu/rebalance/placement_policy_util.cc: http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc@458 PS7, Line 458: } // namespace tools > warning: namespace 'rebalance' ends with a comment that refers to a wrong n Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc File src/kudu/tools/rebalancer_tool.cc: http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@79 PS7, Line 79: using std::multimap; > warning: using decl 'multimap' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@80 PS7, Line 80: using std::numeric_limits; > warning: using decl 'numeric_limits' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@82 PS7, Line 82: using std::set_difference; > warning: using decl 'set_difference' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@99 PS7, Line 99: Fixed -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 22 Aug 2019 22:06:20 + Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#8). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,882 insertions(+), 1,606 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/8 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 8 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc
File src/kudu/rebalance/placement_policy_util-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@33
PS7, Line 33: #include "kudu/consensus/quorum_util.h"
> Hrm.. I might be missing something; why do we need this now?
oops
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@199
PS7, Line 199: Re
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc@397
PS7, Line 397: ksck
> nit: maybe just "health report"? Same below. Or omit details of "ksck" in g
ah, these were copied comments from the original tools/rebalancer.cc
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc@86
PS8, Line 86: }
> To leverage move semantics, pass 'config' to by copy, ie
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@339
PS7, Line 339: const ClusterRawInfo& raw_info,
: const ClusterInfo&
ci,
: ostream& out) const
{
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@750
PS7, Line 750:bool*
timed_out) {
> nit: spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 23:37:06 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#9). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,884 insertions(+), 1,610 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/9 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 9 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health
of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication
factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is
unavailable for
: // writes. If a majority cannot be brought back online, then
the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs
and the master's.
: CONSENSUS_MISMATCH,
: };
> I'm not sure this fits exactly under 'rebalance' namespace. The same conce
I agree, putting these under a namespace 'cluster' makes sense.
Where do you think it should go, in terms of subdirectories in src/kudu?
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Aug 2019 20:27:06 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc
File src/kudu/rebalance/rebalance-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc@182
PS9, Line 182: return HasSameContents(lhs.servers_by_replica_count,
:rhs.servers_by_replica_count);
> nit: indentation
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@19
PS9, Line 19: #include
: #include
> nit here and elsewhere when using IWYU suggestions: please use C++-style in
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
> Add short documentation for the class. The important point is to explain w
A Rebalancer object is a member variable of the class
master::AutoRebalancerTask, but is the base class of tools:RebalancerTool.
RebalancerTool object has some additional functions for printing balance
information.
AutoRebalancerTask is a thread that manages the Rebalancer object.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc@57
PS9, Line 57: std::vector ignored_tservers_param,
: std::vector master_addresses,
: std::vector table_filters,
> nit: remove std:: namespace prefix since there are corresponding 'using std
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@714
PS9, Line 714:
> nit: indent/spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@746
PS9, Line 746:
> nit: indent/spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@781
PS9, Line 781:
> nit: indent/spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 25 Aug 2019 16:57:48 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health
of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication
factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is
unavailable for
: // writes. If a majority cannot be brought back online, then
the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs
and the master's.
: CONSENSUS_MISMATCH,
: };
> I'm OK with leaving it currently under src/kudu/rebalancer. You can shop a
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
> Great! Could you add the gist of it this in the form of a short comments f
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 26 Aug 2019 15:57:56 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#10). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,908 insertions(+), 1,620 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/10 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 10 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#11). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,914 insertions(+), 1,629 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/11 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 11 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
..
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/cluster_status.cc
File src/kudu/rebalance/cluster_status.cc:
PS10:
> We chatted about this offline: there is already a cluster namespace; maybe
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc
File src/kudu/rebalance/rebalance_algo-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc@44
PS10, Line 44:
: namespace kudu {
: namespace rebalance {
: struct TestClusterConfig;
: } // namespace rebalance
: } // namespace kudu
> Can this be moved into the below rebalance namespace?
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h@148
PS10, Line 148: const std::vector&
: summaries,
> nit: could you keep these on the same line?
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc@714
PS10, Line 714:
> nit: spacing here and below
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 10
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 27 Aug 2019 01:21:42 +
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#12). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,918 insertions(+), 1,627 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/12 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 12 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#13). Change subject: Create kudu/rebalance subdirectory .. Create kudu/rebalance subdirectory Moved and renamed structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved tools/rebalance_algo to rebalance/rebalance_algo. Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer. Renamed tools/rebalancer to tools/rebalancer_tool. Created RebalancerTool in tools/rebalancer_tool that inherits from Rebalancer. Moved corresponding tests into kudu/rebalance subdirectory. Movement of code done to prepare for autorebalancer task in master. No functional changes made. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/placement_policy_util-test.cc R src/kudu/rebalance/placement_policy_util.cc R src/kudu/rebalance/placement_policy_util.h R src/kudu/rebalance/rebalance-test.cc R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h A src/kudu/rebalance/rebalancer.cc A src/kudu/rebalance/rebalancer.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h D src/kudu/tools/rebalancer.h R src/kudu/tools/rebalancer_tool.cc A src/kudu/tools/rebalancer_tool.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 26 files changed, 1,918 insertions(+), 1,627 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/13 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 13 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14177 Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. WIP KUDU-2780: create thread for auto-rebalancing Created auto-rebalancer thread in background tasks of catalog manager. Set up framework for auto-rebalancing loop. Loop retrieves information on tservers, tables, and tablets for rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 TODO: calculate rebalancing moves, execute moves, write tests. --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h 8 files changed, 655 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/1 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. Patch Set 1: (13 comments) http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer-test.cc@18 PS1, Line 18: #include "kudu/master/autorebalancer.h" > error: 'kudu/master/autorebalancer.h' file not found [clang-diagnostic-erro Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@48 PS1, Line 48: #include "kudu/util/flags.h" > warning: #includes are not sorted properly [llvm-include-order] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@61 PS1, Line 61: using kudu::rebalance::ClusterBalanceInfo; > warning: using decl 'ClusterBalanceInfo' is unused [misc-unused-using-decls Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@67 PS1, Line 67: using std::endl; > warning: using decl 'endl' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@68 PS1, Line 68: using std::map; > warning: using decl 'map' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@69 PS1, Line 69: using std::numeric_limits; > warning: using decl 'numeric_limits' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@70 PS1, Line 70: using std::pair; > warning: using decl 'pair' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@71 PS1, Line 71: using std::set; > warning: using decl 'set' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@72 PS1, Line 72: using std::shared_ptr; > warning: using decl 'shared_ptr' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@74 PS1, Line 74: using std::to_string; > warning: using decl 'to_string' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@75 PS1, Line 75: using std::unordered_map; > warning: using decl 'unordered_map' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/auto_rebalancer.cc@108 PS1, Line 108:string master_addresses_str) > warning: the parameter 'master_addresses_str' is copied for each invocation Done http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14177/1/src/kudu/master/catalog_manager.cc@788 PS1, Line 788: auto_rebalancer_.reset(new AutoRebalancerTask(this, master_addresses)); > warning: 'master_addresses' used after it was moved [bugprone-use-after-mov Done -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 04 Sep 2019 23:23:24 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#2). Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. WIP KUDU-2780: create thread for auto-rebalancing Created auto-rebalancer thread in background tasks of catalog manager. Set up framework for auto-rebalancing loop. Loop retrieves information on tservers, tables, and tablets for rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 TODO: calculate rebalancing moves, execute moves, write tests. --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h 8 files changed, 646 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/2 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer-test.cc@31 PS2, Line 31: using kudu::cluster::ExternalMiniClusterOptions; > warning: using decl 'ExternalMiniClusterOptions' is unused [misc-unused-usi Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer-test.cc@32 PS2, Line 32: using std::unique_ptr; > warning: using decl 'unique_ptr' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.h File src/kudu/master/auto_rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.h@66 PS2, Line 66: explicit > nit: I don't think explicit is needed here since it's hard to write somethi Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@165 PS2, Line 165: if (!catalog_manager_) continue; > How could it happen that catalog_manager_ is nullptr? And if so, wouldn't Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@168 PS2, Line 168: CatalogManager::ScopedLeaderSharedLock l(catalog_manager_); > As it's written now, the scope of this lock is too wide -- it will take a w Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@171 PS2, Line 171: // TODO(hannah.nguyen): holding onto this lock too long? : // Concerning especially if replica moves aren't async. > Since this lock is used only to synchronize access to closing_ by this meth Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@183 PS2, Line 183: (!BuildClusterRawInfo(boost::none, &raw_info).ok()) > Does it deserve some logging if it was not possible to get a snapshot of th Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@190 PS2, Line 190: \ > drop Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@210 PS2, Line 210: > nit: this extra line isn't good for readability Done http://gerrit.cloudera.org:8080/#/c/14177/2/src/kudu/master/auto_rebalancer.cc@246 PS2, Line 246: > nit here and 3 closing parenthesis above: an extra empty line Done -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sun, 08 Sep 2019 01:00:19 + Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#3). Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. WIP KUDU-2780: create thread for auto-rebalancing Created auto-rebalancer thread in background tasks of catalog manager. Set up framework for auto-rebalancing loop. Loop retrieves information on tservers, tables, and tablets for rebalancing. The number of replica moves per loop iteration is controlled by a flag. If there are placement policy violations, the current loop iteration will only perform replica moves to reinstate the policy. Otherwise, the auto-rebalancer will perform moves to do inter-location(cross-location), then intra-location(by table then by tserver) rebalancing. If the cluster is balanced, the current rebalancing cycle completes, and the thread will sleep for an interval, controlled by another flag. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 TODO: write tests. --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,297 insertions(+), 53 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/3 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 3 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing
..
Patch Set 3:
(40 comments)
http://gerrit.cloudera.org:8080/#/c/14177/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14177/3//COMMIT_MSG@8
PS3, Line 8:
: Created auto-rebalancer thread in background tasks of catalog
manager.
: Set up framework for auto-rebalancing loop.
: Loop retrieves information on tservers, tables, and tablets for
rebalancing.
: The number of replica moves per loop iteration is controlled by a
flag.
: If there are placement policy violations, the current loop
iteration will only
: perform replica moves to reinstate the policy.
: Otherwise, the auto-rebalancer will perform moves to do
inter-location(cross-location),
: then intra-location(by table then by tserver) rebalancing.
: If the cluster is balanced, the current rebalancing cycle
completes, and the
: thread will sleep for an interval, controlled by another flag.
> nit: I found this a little difficult to read. Could you split it up into pa
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@58
PS3, Line 58: if (cluster_)
> nit: it's a good practice to use scope braces for 'if ()' clause even it's
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@61
PS3, Line 61: else return -1;
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@68
PS3, Line 68: else return -1;
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@85
PS3, Line 85: int kNumMasters = 3;
: int kNumTservers = 3;
: int kNumTablets = 4;
> nit: these are constants, right? Add const/constexpr if so.
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@96
PS3, Line 96: 10
> 10+ seconds run-time for a test makes it a 'slow' test. Add a notion of ru
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@101
PS3, Line 101: if (i==leader_idx) ASSERT_NE(0,
number_of_loop_iterations(i));
: else ASSERT_EQ(0, moves_scheduled_this_round(i));
> Would this test fail if master leadership changes while the test is running
Hmm, I think it would.
Is it enough to just reset the leader index then restart the for loop? ie:
if (leader_changed) { leader_idx = new_leader_idx; i=0; continue; }
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@104
PS3, Line 104:
> nit: an extra space
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@111
PS3, Line 111: int kNumMasters = 3;
: int kNumTservers = 3;
: int kNumTablets = 3;
> Add const/constexpr.
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@132
PS3, Line 132: auto iterations = number_of_loop_iterations(new_leader_idx);
: ASSERT_LT(0, iterations);
> Might it happen that the rebalancing thread hasn't yet after master leader
Would it make sense to sleep for a bit to make sure the auto-rebalancer thread
has been initialized and begins iterating?
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@139
PS3, Line 139: kNumTservers, kNumTablets;
> These are not constants, but 'kXxx' is used only for constants. Change the
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@214
PS3, Line 214: auto-rebalancing flag is paused
> nit: how does one pause a flag?
How do you allow a flag to be changed via the CLI?
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@241
PS3, Line 241: ASSERT_GE(moves_scheduled_before, moves_scheduled_after);
> If there shouldn't be any moves, then why ASSERT_GE(), not ASSERT_EQ()?
if the flag to pause auto-rebalancing is set, the loop is still active (ie
checking cluster status), just not getting or executing replica moves.
It's possible that while auto-rebalancing is paused, the cluster becomes
balanced. In this case, the variable "moves_scheduled_this_round" will be reset
to 0. So it's possible that moves_scheduled_after < moves_scheduled_before.
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@243
PS3, Line 243: FLAGS_auto_rebalancing_stop_flag = orig_flag;
> I don't think this is neces
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing
..
Patch Set 3:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@238
PS3, Line 238:
SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds));
> I agree sleeping makes sense, otherwise (in case GetMoves() or ExecuteMoves
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@271
PS3, Line 271: Status AutoRebalancerTask::GetMoves(const ClusterRawInfo&
raw_info, const ClusterInfo& ci, vector*
replica_moves, const TabletsPlacementInfo& placement_info, bool* policy_fixing)
const {
> nit: wrap this. Same elsewhere.
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@279
PS3, Line 279:
: if (ts_id_by_location.size() == 1) {
: rebalance::TwoDimensionalGreedyAlgo algo;
: RETURN_NOT_OK(GetMovesUsingRebalancingAlgo(raw_info,
replica_moves, &algo));
: *policy_fixing = false;
: } else {
:
: if (rebalancer_->ShouldRunPolicyFixer()) {
: vector ppvi;
:
RETURN_NOT_OK(DetectPlacementPolicyViolations(placement_info, &ppvi));
: // Filter out all reported violations which are already
taken care of.
: RETURN_NOT_OK(FindMovesToReimposePlacementPolicy(
: placement_info, ci.locality, ppvi, replica_moves));
: *policy_fixing = true;
: }
:
: if (replica_moves->empty()) {
: if (rebalancer_->ShouldRunCrossLocationRebalancing()) {
: rebalance::LocationBalancingAlgo algo(1.0);
: RETURN_NOT_OK(GetMovesUsingRebalancingAlgo(
: raw_info, replica_moves, &algo,
/*cross_location=*/true));
: }
:
: if (rebalancer_->ShouldRunIntraLocationRebalancing() &&
replica_moves->size() < max_moves) {
: rebalance::TwoDimensionalGreedyAlgo algo;
: for (const auto& elem : ts_id_by_location) {
: const auto& location = elem.first;
: ClusterRawInfo location_raw_info;
: BuildClusterRawInfo(location, &location_raw_info);
: RETURN_NOT_OK(GetMovesUsingRebalancingAlgo(
: location_raw_info, replica_moves, &algo));
: }
: }
: *policy_fixing = false;
: }
: }
:
: return Status::OK();
: }
> nit: This could use some comments.
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@377
PS3, Line 377: std::shuffle(tablet_ids.begin(), tablet_ids.end(),
random_device());
: string move_tablet_id;
: for (const auto& tablet_id : tablet_ids) {
: if (tablets_in_move.find(tablet_id) ==
tablets_in_move.end()) {
: // For now, choose the very first tablet that does not
have replicas
: // in move. Later on, additional logic might be added to
find
: // the best candidate.
: move_tablet_id = tablet_id;
: break;
: }
: }
: if (move_tablet_id.empty()) {
: return Status::NotFound(Substitute(
: "table $0: could not find any suitable replica to move
"
: "from server $1 to server $2", move.table_id,
move.from, move.to));
: }
: Rebalancer::ReplicaMove move_info;
: move_info.tablet_uuid = move_tablet_id;
: move_info.ts_uuid_from = move.from;
: const auto& extra_info = FindOrDie(extra_info_by_tablet_id,
move_tablet_id);
: if (extra_info.replication_factor < extra_info.num_voters) {
: // The number of voter replicas is greater than the target
replication
: // factor. It might happen the replica distribution would
be better
: // if just removing the source replica. Anyway, once a
replica is removed,
: // the system will automatically add a new one, if needed,
where t
> Seems like a good chunk of this is taken from rebalancer_tool.cc. Can we re
Done
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@439
PS3,
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#4). Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. WIP KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 TODO: write tests. --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 10 files changed, 1,305 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/4 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 4 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing
..
Patch Set 4:
(2 comments)
some straggler comment responses
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@377
PS3, Line 377: unordered_set tablets_in_move;
: for (const auto& move : moves) {
: vector tablet_ids;
: RETURN_NOT_OK(rebalancer_->FindReplicas(move, raw_info,
&tablet_ids));
: if (cross_location) {
: // In case of cross-location (a.k.a. inter-location)
rebalancing it is
: // necessary to make sure the majority of replicas would
not end up
: // at the same location after the move. If so, remove
those tablets
: // from the list of candidates.
:
RETURN_NOT_OK(rebalancer_->FilterCrossLocationTabletCandidates(
: cluster_info.locality.location_by_ts_id, tpi, move,
&tablet_ids));
: }
: // Shuffle the set of the tablet identifiers: that's to
achieve even spread
: // of moves across tables with the same skew.
: std::shuffle(tablet_ids.begin(), tablet_ids.end(),
random_device());
: string move_tablet_id;
: for (const auto& tablet_id : tablet_ids) {
: if (tablets_in_move.find(tablet_id) ==
tablets_in_move.end()) {
: // For now, choose the very first tablet that does not
have replicas
: // in move. Later on, additional logic might be added to
find
: // the best candidate.
: move_tablet_id = tablet_id;
: break;
: }
: }
> Done
Oops, actual response:
This is pulled from just part of
RebalancerTool::AlgoBasedRunner::GetNextMovesImpl().
The original function has to deal with the possibility of unhealthy
tservers/tablets, filtering out the tservers and moves that are already
scheduled/in flight.
I didn't know if it was worth pulling out 30ish lines of code out of a member
function of a class in a different namespace--do you think it's worth
refactoring?
http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@452
PS3, Line 452: }
> We called CatalogManager::GetTabletLocations() when we computed the cluster
In AutoRebalancerTask::BuildClusterRawInfo(),
CatalogManager::GetTabletLocations() is called once for every single tablet in
the cluster.
Here, we're calling only calling it again for the tablets that are involved in
the specified replica_moves.
We won't know in BuildClusterRawInfo() which tablets will fall in that subset,
so I didn't think it was worth stashing the information for all the tablets.
Plus, the size of replica_moves is limited too.
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 4
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 19 Sep 2019 20:55:58 +
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#5). Change subject: WIP KUDU-2780: create thread for auto-rebalancing .. WIP KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 TODO: write tests. --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 10 files changed, 1,306 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/5 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 5 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing
..
Patch Set 5:
(64 comments)
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@50
PS5, Line 50: FLAGS_auto_rebalancing_stop_flag
> You probably meant to set the interval here.
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@87
PS5, Line 87:
: if (!AllowSlowTests()) {
: LOG(WARNING) << "test is skipped; set KUDU_ALLOW_SLOW_TESTS=1
to run";
: return;
: }
> nit: you can use SKIP_IF_SLOW_NOT_ALLOWED() for this. Same elsewhere.
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@111
PS5, Line 111: }
: else {
> Join the lines for better readability.
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@104
PS5, Line 104: SleepFor(MonoDelta::FromSeconds(10));
:
: int leader_idx;
: ASSERT_OK(cluster_->GetLeaderMasterIndex(&leader_idx));
: for (int i=0; i Rather than sleeping this long, how about asserting with ASSERT_EVENTUALLY
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@117
PS5, Line 117: cluster_->Shutdown();
> nit: don't need this since it gets called in TearDown()
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@144
PS5, Line 144: auto iterations = number_of_loop_iterations(new_leader_idx);
: ASSERT_LT(0, iterations);
> Should this be wrapped in ASSERT_EVENTUALLY? What if the thread hasn't star
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@149
PS5, Line 149: TEST_F(AutoRebalancerTest, NoReplicaMovesIfBalanced) {
> Consider breaking this into smaller tests so the cases are parallelizable.
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@200
PS5, Line 200: num_tservers = 2;
: num_tablets = 1;
> It doesn't seem particularly useful to keep defining these separately, and
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@224
PS5, Line 224: SleepFor(MonoDelta::FromSeconds(10));
: ASSERT_OK(cluster_->GetLeaderMasterIndex(&leader_idx));
: ASSERT_EQ(moves_scheduled_this_round(leader_idx), 0);
> I wonder if it's sufficient to just define "no moves" as having the loop it
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer-test.cc@263
PS5, Line 263: ASSERT_GE
> ASSERT_EQ()?
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h@63
PS5, Line 63: // the catalog manager must be in the process of initializing.
> nit: Why does this need to be the case? The only invariant I see right now
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h@71
PS5, Line 71: int number_of_loop_iterations;
: int moves_scheduled_this_round;
> Nit: could you suffix these with _for_test so it's obvious that their acces
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h@70
PS5, Line 70: // Variables for testing.
: int number_of_loop_iterations;
: int moves_scheduled_this_round;
> Is it possible to make this variables private and declare the test that acc
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h@85
PS5, Line 85: const boost::optional& location
> Please document the semantics of the 'location' parameter -- it's not self
Done
http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.h@88
PS5, Line 88: Status GetMovesUsingRebalancingAlgo(
: const rebalance::ClusterRawInfo& raw_info,
: std::vector*
replica_moves,
: rebalance::RebalancingAlgo* algo,
: bool cross_location = false) const;
:
: Status
GetMoves(std::vector* replica_moves,
: bool* policy_fixing) const;
:
: Status GetTabletLeader(
: const std::string& tablet_id,
: std::string* leader_uuid,
: HostPort* leader_hp) const;
:
: Status ExecuteMoves(
: const std::vector&
replica_moves,
: const bool& policy_fixing);
> nit: add docs. What do these and thei
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#6). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,308 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/6 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 6 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#7). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,327 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/7 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#8). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,333 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/8 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 8 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#9). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,333 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/9 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 9 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#10). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,332 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/10 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 10 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#11). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tservers, tables, and tablets, in order to determine the best rebalancing replica moves for the cluster. The loop will continue, determining and executing a certain number of moves per iteration. The number of replica moves per server, per loop iteration, is controlled by a flag. If the cluster has placement policy violations, the thread will schedule and execute replica moves to reinstate the policy. Otherwise, the thread will perform inter-location(cross-location), then intra-location(by table, then by tserver) rebalancing. If the thread finds that the cluster is already balanced, the current rebalancing cycle completes. The thread will sleep for a time interval, controlled by a flag. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc 11 files changed, 1,334 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/11 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 11 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 11:
(27 comments)
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer-test.cc@165
PS11, Line 165: // Each tserver is considered its own location.
> I'm not sure I understand what this means. As far as I know, in case of no
Ah, misunderstood the testing setup. Will add in explicit location assignment
for tablet servers--thank you for the code pointer!
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer-test.cc@180
PS11, Line 180: Each tserver is its own location
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.h@165
PS11, Line 165: // Structs to hold cluster information.
: // rebalance::ClusterRawInfo raw_info_;
: // rebalance::ClusterInfo cluster_info_;
: // rebalance::TabletsPlacementInfo placement_info_;
> Remove if no longer needed?
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@146
PS11, Line 146: 300
> Is this used by the rebalancer thread?
No, it's not. This is the default value assigned in the rebalancer tool. Should
I set it to something else or specify that it's unused?
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@224
PS11, Line 224:
SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds));
> Would this be a double-pause given there is another sleep at line 195?
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@228
PS11, Line 228: Rebalancer::MovesInProgress()
> Why the moves in progress are empty? I think this cannot be empty: the inf
Ah, I initially intended for the replica movement to be synchronous (ie the
loop iteration would wait for the moves to complete) but as implemented, will
need to keep track of moves between iterations and do something similar to (or
refactor and utilize) Rebalancer::FilterMoves() and variants of
UpdateMovesInProgressStatus() in RebalancerTool.
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@231
PS11, Line 231:
SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds));
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@236
PS11, Line 236: Rebalancer::MovesInProgress()
> Ditto: why is it empty? What if previous run has scheduled some moves? Ma
Responded above.
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@239
PS11, Line 239:
SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds)
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@246
PS11, Line 246: IsClusterBalanced
> Why this method/function is necessary at all? Wouldn't GetMoves() return a
Yes, GetMoves() will return an empty set if the cluster is balanced.
But I thought that it's also possible that GetMoves() could return an empty set
of moves if there are errors in retrieving information about the cluster at
some point, eg calls to BuildClusterRawInfo() for each location in
intra-location rebalancing. In this case, there shouldn't be any moves
scheduled and the task sleeps, but the cluster isn't necessarily balanced, so
the round isn't complete.
I've used the result of IsClusterBalanced() to differentiate between completed
rounds of rebalancing and resetting moves_scheduled_this_round_for_test_ to 0.
Is using it for testing enough to justify keeping the function?
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@249
PS11, Line 249: lock_guard l(lock_for_test_);
: moves_scheduled_this_round_for_test_ = 0;
> nit: might be worth just using an atomic or something
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@252
PS11, Line 252:
SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds));
> Don't need this anymore, right? We'll sleep up top? Same below.
Done
http://gerrit.cloudera.org:8080/#/c/14177/11/src/kudu/master/auto_rebalancer.cc@298
PS11, Line 298: // One location: use greedy rebalancing algorithm to find
moves.
: if (ts_id_by_location.size() == 1) {
: rebalance::TwoDimensionalGreedyAlgo algo;
: RETURN_NOT_OK(GetMovesUsingRebalancingAlgo(raw_info, &
[kudu-CR] util: fix macOS build of thread.cc
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14516 ) Change subject: util: fix macOS build of thread.cc .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddb61d9a933a9b76ec73fbfd0a4ef555bc27786d Gerrit-Change-Number: 14516 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 20 Oct 2019 16:32:07 + Gerrit-HasComments: No
[kudu-CR] KUDU-2914: Rebalance tool support moving replicas from some specific tablet servers
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14154 ) Change subject: KUDU-2914: Rebalance tool support moving replicas from some specific tablet servers .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/14154/10/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14154/10/src/kudu/rebalance/rebalancer.h@70 PS10, Line 70: If empty, run the rebalancing on : // all tablet servers in the cluster. Would it be beneficial to have the option to choose the former default behavior of the rebalancer tool to not do rebalancing if there are any or some specified number of unhealthy tservers? What if there are enough healthy tservers to maintain the RF and run rebalancing, but there are also a significant number of unhealthy tservers? Could the rebalancer tool end up moving replicas that will make the cluster imbalanced, and have to be moved again, once the unhealthy tservers become available again? http://gerrit.cloudera.org:8080/#/c/14154/10/src/kudu/tools/rebalancer_tool.cc File src/kudu/tools/rebalancer_tool.cc: http://gerrit.cloudera.org:8080/#/c/14154/10/src/kudu/tools/rebalancer_tool.cc@631 PS10, Line 631: CheckIgnoredServers Is this function only called when move_replicas_from_ignored_tservers=true? When move_replicas_from_ignored_tservers=false, Rebalancer::BuildClusterInfo() still populates ignored_tservers with unhealthy tservers. We should still verify that remaining_tservers_count >= max_replication_factor, even if not moving replicas from the ignored tservers. -- To view, visit http://gerrit.cloudera.org:8080/14154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I86cfb740030946c13db1a9ca63d241f4907d6c89 Gerrit-Change-Number: 14154 Gerrit-PatchSet: 10 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Thu, 24 Oct 2019 16:10:18 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#12). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. Currently, one move is scheduled per iteration and executed synchronously. The thread waits for the move to complete, then sleeps for a time interval, controlled by a flag. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_main.cc M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 17 files changed, 1,565 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/12 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 12 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#13). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. Currently, one move is scheduled per iteration and executed synchronously. The thread waits for the move to complete, then sleeps for a time interval, controlled by a flag. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,560 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/13 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 13 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 12:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@103
PS12, Line 103: using std::multimap;
> warning: using decl 'multimap' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@104
PS12, Line 104: using std::random_device;
> warning: using decl 'random_device' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@105
PS12, Line 105: using std::set;
> warning: using decl 'set' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@109
PS12, Line 109: using std::transform;
> warning: using decl 'transform' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@110
PS12, Line 110: using std::unique_ptr;
> warning: using decl 'unique_ptr' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@168
PS12, Line 168: FLAGS_auto_rebalancing_load_imbalance_threshold))),
> warning: narrowing conversion from 'double' to 'bool' [bugprone-narrowing-c
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@363
PS12, Line 363: int max_moves = FLAGS_auto_rebalancing_max_moves_per_server *
num_tservers;
> warning: narrowing conversion from 'unsigned int' to signed type 'int' is i
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@383
PS12, Line 383: rebalancer_.FindReplicas(move, raw_info, &tablet_ids);
> warning: static member accessed through instance [readability-static-access
Done
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@701
PS12, Line 701: while (moves_to_remove.size() != 0) {
> warning: the 'empty' method should be used to check for emptiness instead o
Done
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 12
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 03 Feb 2020 19:48:39 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 13:
(40 comments)
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@64
PS13, Line 64: void AssignLocations(int num_locations, int num_tservers) {
> nit: could you add a comment explaining that this doesn't assign locations
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@68
PS13, Line 68: TSDescriptorVector descs;
: cluster_->mini_master(master_idx)->master()->ts_manager()->
: GetAllDescriptors(&descs);
> Probably want to put this into the ASSERT_EVENTUALLY too? Right now it's ju
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@76
PS13, Line 76: // Assign num_locations unique locations to the first
num_locations tservers.
: for (int i = 0; i < num_locations; ++i) {
: descs[i]->AssignLocationForTesting(Substitute("L$0", i));
: }
> What if num_locations turns to be greater than num_tservers? Most likely t
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@93
PS13, Line 93: moves_scheduled_this_round_for_test(leader_idx), 0
> nit: in ASSERT_EQ(), the expected value comes first -- that way it's easier
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@106
PS13, Line 106: number_of_loop_iterations_for_test
> nit: use PascalCase for these, since they're not simple member accessors.
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@111
PS13, Line 111: return -1;
> nit: maybe instead of doing this, if we know we'll always have a cluster, d
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@150
PS13, Line 150: nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@151
PS13, Line 151: i==lead
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@153
PS13, Line 153: NE
> Could be even more explicit and say ASSERT_LT, right? Especially if -1 is a
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@189
PS13, Line 189: number_of_loop_iterations_for_test
> Maybe do a sanity check before shutting down the leader checking that this
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@196
PS13, Line 196: skewed
> nit: no longer balanced
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@208
PS13, Line 208: SleepFor(MonoDelta::FromSeconds(1));
> Here and below: is there a more reliable way of knowing the auto-rebalancer
Created function CheckAutoRebalancerStarted() with the above functionality,
thank you!
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@258
PS13, Line 258: locations.size(), kNumTservers
> nit here and elsewhere: the expected value comes first in {ASSERT,EXPECT}_E
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@260
PS13, Line 260: TestWorkload workload(cluster_.get());
: workload.set_num_tablets(kNumTablets);
: workload.set_num_replicas(1);
: workload.Setup();
> This pattern repeats in a few places. Maybe, separate this into a method/f
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@268
PS13, Line 268: If placement policy can never be satisfied, the cluster is
balanced
> This statement is too bold. :)
Done
http://gerrit.cloudera.org:8080/#/c/14177/13/src/kudu/master/auto_rebalancer-test.cc@300
PS13, Line 300: } // namespace master
> After looking into the implementation of AutoRebalancerTask::CheckReplicaMo
I added a DCHECK() in RunLoop() of auto_rebalancer.cc to verify that the
auto-rebalancer won't schedule a new batch of replica moves until the previous
batch's moves have all completed.
Will add a test with the above scenario to verify that the number of tablet
copying sessions per tablet server doesn't exceed the specified
FLAGS_auto_rebalancing_max_moves_per_server value, thank you!
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@220
PS12, Line 220: moves_scheduled_this_round_for_test_ = 0;
> Does this mean that replica_moves might not be empty? And if it's not empty
Clarified this in comment
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#14). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,624 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/14 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 14 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 14:
(18 comments)
Still need to look into implementing a condition variable waiting vs. sleeping
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc@159
PS14, Line 159: shared_ptr
> nit: any special requirements to necessitate usage of std::shared_ptr? If
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc@161
PS14, Line 161: shared_ptr
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc@329
PS14, Line 329: if (!AllowSlowTests()) {
: LOG(WARNING) << "test is skipped; set
KUDU_ALLOW_SLOW_TESTS=1 to run";
: return;
: }
> nit: consider using SKIP_IF_SLOW_NOT_ALLOWED macro instead
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc@345
PS14, Line 345: SetupWorkLoad(kNumTablets, /*num_replicas*/3);
> I suspect this fails if number of tablet servers in cluster is less than 3,
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer-test.cc@369
PS14, Line 369: for (auto replica : replicas) {
> warning: loop variable is copied but only used as const reference; consider
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@209
PS14, Line 209: if (paused_) continue;
> Is this set anywhere? If not, remove it?
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@273
PS14, Line 273: all_moves_complete
> Do we need this? Could we just check if replica_moves is empty?
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@660
PS14, Line 660: *all_moves_complete = true;
> nit: it's generally best practice to not update out-params if the method fa
Actually just removed this param; can just check size of replica_moves per your
comment above, thanks :)
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@663
PS14, Line 663: stack moves_to_remove;
> Is the LIFO order here important? And is it important to actually pop every
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@670
PS14, Line 670: move_completion_status
> Not used? Is that intentional?
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/ts_manager.cc
File src/kudu/master/ts_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/ts_manager.cc@214
PS14, Line 214: for (const TSDescriptorMap::value_type& entry :
servers_by_id_) {
> nit: in C++11 you can use auto for this, e.g.
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.h@171
PS14, Line 171: The source and destination replicas are determined by the
elements of the
: // 'tablet_ids' container and tablet server UUIDs
TableReplicaMove::from and
: // TableReplica::to correspondingly.
> nit: not from this patch, but this makes it seem like 'tablet_ids' is an in
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.h@260
PS14, Line 260: '
> nit: remove
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.h@261
PS14, Line 261: in move
> nit: it's easy to misread this as "replicas in 'move'". Maybe "replicas in
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.h@264
PS14, Line 264: Status SelectReplicasToMove(
> nit: maybe also describe the behavior re: potentially returning an empty st
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.cc@487
PS14, Line 487: const TableReplicaMove& move,
: const unordered_map&
extra_info_by_tablet_id,
: std::mt19937 random_generator,
: vector tablet_ids,
: unordered_set* tablets_in_move,
: vector* replica_moves) {
> nit: indent by four spaces
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.cc@497
PS14, Line 497: string
> nit: make this a const ref?
Can't declare a reference unless I initialize it, which has to wait until the
for loop below
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/tools/rebalancer_tool.cc
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#15). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,608 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/15 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 15 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#16). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,665 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/16 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 16 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 16:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@732
PS12, Line 732: string orig_leader_uuid;
: HostPort orig_leader_hp;
: RETURN_NOT_OK(GetTabletLeader(tablet_uuid, &orig_leader_uuid,
&orig_leader_hp));
: shared_ptr proxy;
: shared_ptr desc;
: if (!ts_manager_->LookupTSByUUID(orig_leader_uuid, &desc)) {
: return Status::NotFound("Could not find destination
tserver");
: }
: RETURN_NOT_OK(desc->GetConsensusProxy(messenger_, &proxy));
:
: // Check if replica at 'to_ts_uuid' is in the config, and if
it has been
: // promoted to voter.
: ConsensusStatePB cstate;
: GetConsensusStateRequestPB req;
: GetConsensusStateResponsePB resp;
: RpcController rpc;
:
rpc.set_timeout(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_timeout_seconds));
: req.set_dest_uuid(orig_leader_uuid
> Yeah I'm fine improving this later. Could you add a TODO pointing to the ot
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@207
PS14, Line 207: // If not the leader, don't do anything.
> I think it's better to use ConditionVariable::WaitFor(FLAGS_auto_rebalancin
Done
http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@271
PS14, Line 271: if (closing_) {
> I think it make sense to check for 'closing_' while waiting for the replica
Done
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 16
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 20 Feb 2020 22:52:04 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#17). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,661 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/17 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 17 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 17:
(26 comments)
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer-test.cc@81
PS17, Line 81: &
> Having it written as it's now, it hints on the expectation of the number of
Done. Removed the similar reference below in AssignLocationsWithSkew() as well.
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer-test.cc@97
PS17, Line 97: %
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer-test.cc@359
PS17, Line 359: 1000
> Given the exponential back-off behavior of ASSERT_EVENTUALLY under CheckSom
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@117
PS17, Line 117: Status GetMoves(
> nit: can you also note how this can fail?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@132
PS17, Line 132: Marks replicas that
> nit: marks them with what?
Done. Updated comment.
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@137
PS17, Line 137: Status ExecuteMoves(
> nit: can you also note how this can fail?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@139
PS17, Line 139: const bool& has_location_violations
> nit: booleans are trivially copyable so we don't need to pass them by const
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@178
PS17, Line 178: mutable
> Is 'mutable' is really needed here?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.h@188
PS17, Line 188: wake_up_cv_
> It wasn't clear to me that this is only used to signal that the rebalancer
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@199
PS17, Line 199: // Check if shutdown was signaled.
: {
: lock_guard l(lock_);
: if (closing_) return;
: }
:
> nit: maybe this should be
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@207
PS17, Line 207: CatalogManager::ScopedLeaderSharedLock
l(catalog_manager_);
: if (!l.first_failed_status().ok()) {
: moves_scheduled_this_round_for_test_ = 0;
: continue;
: }
> Should we at least sleep here? Otherwise this is a tight loop for followers
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@267
PS17, Line 267: wake_up_cv_.WaitFor(
:
MonoDelta::FromSeconds(FLAGS_auto_rebalancing_wait_for_replica_moves_seconds));
: if (closing_) {
: return;
: }
> This can be:
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@320
PS17, Line 320: // If no placement policy violations are found, do load
rebalancing.
: if (!replica_moves->empty()) {
: return Status::OK();
: }
> nit: move this into the policy fixer block?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@326
PS17, Line 326: *has_location_violations = false;
> nit: update a local variable and swap at the end?
This bool isn't updated anywhere else; should I move the assignment to the end?
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@332
PS17, Line 332: /*cross_location=*/
> nit: don't need this now that the enum is self-documenting
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@343
PS17, Line 343: replica_moves
> nit: update a local variable and swap at the end?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@388
PS17, Line 388: replica_moves
> nit: update a local variable and swap at the end?
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@450
PS17, Line 450: Mark the specified destination tserver,
> nit: what mark are we making?
Done. Changed the comment.
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@526
PS17, Line 526: RETURN_NOT_OK(catalog_manager_->GetAllTables(&table_infos));
> Do we need to RETURN_NOT_OK(leaderlock.first_failed_status())?
BuildClusterRawInfo(
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 17:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@501
PS17, Line 501: TSDescriptorVector descriptors;
:
ts_manager_->GetDescriptorsAvailableForPlacement(&descriptors);
: if (descriptors.size() != ts_manager_->GetLiveCount()) {
: return Status::IllegalState(Substitute("not all tservers
available for tablet placement"));
: }
> The TSManager only knows about tablet servers that have heartbeated recentl
I think this is verified at L600?
The uuids of tservers available for placement (according to the TSManager) are
put into tserver_uuids.
The ReplicaSummary's rep.ts_uuid is filled with the TSInfoPB returned by a call
to CatalogManager::GetTabletLocations().
If the replica is known by the catalog manager but isn't known by the
TSManager, this check should fail:
if (!ContainsKey(tserver_uuids, rep.ts_uuid)) {
return Status::NotFound(Substitute("tserver $0 not available for
placement",
rep.ts_uuid));
}
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@526
PS17, Line 526: RETURN_NOT_OK(catalog_manager_->GetAllTables(&table_infos));
> Right, but leadership can change in between the taking of this lock, e.g. i
Done
http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@570
PS17, Line 570: // Consensus state information is the same for all
replicas of this tablet.
> Yeah we probably shouldn't, if it really isn't used.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 17
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 25 Feb 2020 07:58:23 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: KUDU-2780: create thread for auto-rebalancing .. Patch Set 17: (3 comments) http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/master/auto_rebalancer.cc@261 PS17, Line 261: continue; > If we're moving on without successfully executing and calling CheckReplicaM Changed GetMoves() and added comments to note that 'replica_moves' and 'has_location_violations' will not be updated if GetMoves() fails. The loop should continue and pass the DCHECK next iteration. 'replica_moves' is passed by const ref in ExecuteMoves(), and returns in failure if a move cannot be scheduled. Changing this to WARN_NOT_OK() and proceeding with calling CheckReplicaMovesCompleted() after, which should catch the failure to schedule and remove the move from 'replica_moves'. Also going to add a test with unstable Raft configurations to verify the DCHECK passes, thank you. :) http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.cc File src/kudu/rebalance/rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/rebalance/rebalancer.cc@489 PS14, Line 489: std::mt19937 random_generato > Should this be passed as a pointer since it's an in-out param? Otherwise, i Done http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/rebalance/rebalancer.cc File src/kudu/rebalance/rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/17/src/kudu/rebalance/rebalancer.cc@489 PS17, Line 489: std::mt19937 random_generator, : vector tablet_ids, > It seems Andrew's comment about copying/passing by values hasn't been addre Done. Thanks for catching this--I missed it! -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 17 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sun, 01 Mar 2020 01:01:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#18). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 16 files changed, 1,665 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/18 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 18 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 18:
(35 comments)
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@146
PS18, Line 146: // Make sure the leader master has begun the auto-rebalancing
thread.
: void CheckAutoRebalancerStarted() {
: ASSERT_EVENTUALLY([&] {
: int leader_idx;
: ASSERT_OK(cluster_->GetLeaderMasterIndex(&leader_idx));
: ASSERT_LT(0, NumLoopIterations(leader_idx));
: });
: }
:
: // Make sure the auto-rebalancing loop has iterated a few
times,
: // and no moves were scheduled.
: void CheckNoMovesScheduled() {
: ASSERT_EVENTUALLY([&] {
: int leader_idx;
: ASSERT_OK(cluster_->GetLeaderMasterIndex(&leader_idx));
: ASSERT_LT(3, NumLoopIterations(leader_idx));
: ASSERT_EQ(0, NumMovesScheduled(leader_idx));
: });
: }
:
: void CheckSomeMovesScheduled() {
: ASSERT_EVENTUALLY([&] {
: int leader_idx;
: ASSERT_OK(cluster_->GetLeaderMasterIndex(&leader_idx));
: ASSERT_LT(1, NumLoopIterations(leader_idx));
: ASSERT_LT(0, NumMovesScheduled(leader_idx));
: });
: }
> It isn't important that these calls aren't repeatable is it? At least I don
It doesn't matter with the way these are used in the current set of tests are
written, but I'll update it to the above in case they're used in future tests
and need to be called multiple times in one test.
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@178
PS18, Line 178:
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@184
PS18, Line 184:
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@253
PS18, Line 253:ASSERT_EQ(0, NumLoopIterations(i));
: }
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@269
PS18, Line 269: // Create a cluster that is initially balanced.
: // Bring up another tserver, and verify that moves are
scheduled,
: // since the cluster is no longer balanced.
: TEST_F(AutoRebalancerTest, MovesScheduledIfAddTserver) {
> Do you think it's worth testing that recovering replicas doesn't count towa
If there's a discrepancy between the number of tservers the TSManager believes
are available and the number of tservers the CatalogManager believes are up
with tables on them, then to be safe, the auto-rebalancer just exits the
current iteration and sleeps (exiting BuildClusterRawInfo()), and waits to see
if next cycle it will be consistent.
If the number of tservers is already consistent, but there are some tablets
that are still under-replicated, then those tablets aren't considered,
according to the rebalancing logic that's already present (and been tested) in
BuildTabletsPlacementInfo(), in rebalance/placement_policy_util.
Do you think it's worth adding a test to verify the former situation? Or any
other scenario I might be missing?
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@424
PS18, Line 424: Inject latency.
> nit: Inject latency to make tablet's Raft configuration unstable due to fre
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@429
PS18, Line 429: the latency.
> nit: frequent leadership changes.
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer.h@66
PS18, Line 66: //
> nit: remove the extra line
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer.h@83
PS18, Line 83: friend class CatalogManager;
> nit: Is this still needed?
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer.h@157
PS18, Line 157: nor 'completion_status'
> That doesn't exist anymore.
Done
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer.h@159
PS18, Line 159: 'completion_status' is set to the
: // corresponding move status: Status::OK() if the move
succeeded, or
: // non-OK status if it faile
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#19). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,650 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/19 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 19 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 19:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/14177/18/src/kudu/master/auto_rebalancer-test.cc@269
PS18, Line 269: ASSERT_LT(0, iterations);
: });
: }
:
> >If there's a discrepancy between the number of tservers the TSManager beli
Done
http://gerrit.cloudera.org:8080/#/c/14177/19/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/19/src/kudu/master/auto_rebalancer.cc@391
PS19, Line 391: for (const auto& r : locs_pb.interned_replicas()) {
: if (r.role() == RaftPeerPB::LEADER) {
: int index = r.ts_info_idx();
: const TSInfoPB ts_info =
*(ts_infos_dict.ts_info_pbs[index]);
: *leader_uuid = ts_info.permanent_uuid();
: const auto& addr = ts_info.rpc_addresses(0);
: leader_hp->set_host(addr.host());
: leader_hp->set_port(addr.port());
: break;
: }
: }
: return Status::OK();
> Per the comment methods:
Done
http://gerrit.cloudera.org:8080/#/c/14177/19/src/kudu/master/auto_rebalancer.cc@426
PS19, Line 426: return Status::NotFound("Could not find leader replica's
tserver");
> nit: would be nice to include the leader's UUID in this error message
Done
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 19
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 11 Mar 2020 17:40:10 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#20). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,690 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/20 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 20 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: KUDU-2780: create thread for auto-rebalancing .. Patch Set 20: (2 comments) rify http://gerrit.cloudera.org:8080/#/c/14177/20/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/20/src/kudu/master/auto_rebalancer-test.cc@457 PS20, Line 457: // Verify that movement of replicas to meet the replication factor : // does not count towards rebalancing, i.e. the auto-rebalancer will : // not consider recovering replicas as candidates for replica movement. > BTW, how do we know that the absence of attempts to rebalance is not due to Updated the test accordingly, thank you! http://gerrit.cloudera.org:8080/#/c/14177/20/src/kudu/master/auto_rebalancer-test.cc@462 PS20, Line 462: FLAGS_tserver_unresponsive_timeout_ms > It's a separate topic (i.e. not a topic for this particular scenario), but Added another test case to make sure that the auto-rebalancer handles this scenario as expected. -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 20 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 14 Mar 2020 22:31:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#22). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,734 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/22 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 22 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#23). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,735 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/23 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 23 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#24). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,748 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/24 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 24 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: KUDU-2780: create thread for auto-rebalancing .. Patch Set 24: (4 comments) http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer-test.cc@470 PS23, Line 470: 4 > What if there were 4 tablets? Would the behavior change? Thanks for the suggestion! Changed it to 4 tablets. http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer-test.cc@524 PS23, Line 524: } > Does it make sense to add a post-condition to verify the state of the syste Added this! http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer.cc@201 PS23, Line 201: continue; > +1 Done http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/rebalance/rebalancer.h@266 PS23, Line 266: SelectReplicaToMove( > nit: this is only selecting a single move, so maybe rename it to 'SelectRep Done -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 24 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 17 Mar 2020 05:51:22 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#25). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,757 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/25 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 25 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 ) Change subject: KUDU-2780: create thread for auto-rebalancing .. Patch Set 25: (4 comments) http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/23/src/kudu/master/auto_rebalancer-test.cc@524 PS23, Line 524:"could not perform replica move: Network error"); > Hrm, seems like the new post-condition checks that the rebalancer will try Ah, thanks for the clarification. Hopefully the updated test (and more specific error message) and the post-condition verification makes sense. http://gerrit.cloudera.org:8080/#/c/14177/24/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/24/src/kudu/master/auto_rebalancer-test.cc@528 PS24, Line 528: onds(FLAGS_tserver_unresponsive_timeo > This is 2 minutes, which is pretty long to be sleeping in a test. How about Done http://gerrit.cloudera.org:8080/#/c/14177/24/src/kudu/master/auto_rebalancer-test.cc@528 PS24, Line 528: SleepFor(MonoDelta::FromMilliseconds(FLAGS_tserver_unresponsive_timeout_ms)); > warning: result of integer division used in a floating point context; possi Done http://gerrit.cloudera.org:8080/#/c/14177/24/src/kudu/master/auto_rebalancer-test.cc@528 PS24, Line 528: FromMillise > nit: could use MonoDelta::FromMilliseconds() Done -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 25 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 17 Mar 2020 09:29:12 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing
..
Patch Set 25:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@189
PS25, Line 189: // With the current synchronous implementation, verify that
any moves
: // scheduled in the previous iteration completed.
: DCHECK_EQ(0, replica_moves.size());
> nit: with current state of the code, it seems this DCHECK_EQ() and the comm
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@245
PS25, Line 245: moves_scheduled_this_round_for_test_ = replica_moves.size();
> nit: maybe move this after we actually schedule the moves? ie after Execute
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@247
PS25, Line 247: ExecuteMoves(replica_moves),
> It's only OK for this to be WARN_NOT_OK because any moves that we failed to
Will add a TODO for this, thank you!
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@248
PS25, Line 248: "could not schedule auto-rebalancing replica moves");
> nit: this might be clearer as "failed to send move request" or somesuch.
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@256
PS25, Line 256: "could not perform replica move");
> nit: this might be clearer as "failed to check if move completed" or somesu
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@427
PS25, Line 427: communciate
> communicate
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@656
PS25, Line 656: Status move_completion_status;
> nit: this is unused
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/catalog_manager.cc@810
PS25, Line 810: PREDICT_TRUE
> Remove PREDICT_TRUE: the default value for --auto_rebalancing_enabled is 'f
Done
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/master_runner.cc
File src/kudu/master/master_runner.cc:
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/master_runner.cc@60
PS25, Line 60: // Validates that if auto-rebalancing is enabled, the cluster
uses 3-4-3 replication
: // (the --raft_prepare_replacement_before_eviction flag must be
set to true).
: static Status Validate343SchemeEnabledForAutoRebalancing() {
: if (FLAGS_auto_rebalancing_enabled &&
: !FLAGS_raft_prepare_replacement_before_eviction) {
: return Status::ConfigurationError("If enabling
auto-rebalancing, "
: "Kudu must be configured
with "
: "
--raft_prepare_replacement_before_eviction.");
: }
: return Status::OK();
: }
> Usually this kind of precondition would be enforced via GROUP_FLAG_VALIDATO
Created a validator in master/catalog_manager.cc where the flag is defined,
thank you for the code pointer!
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/ts_descriptor.h
File src/kudu/master/ts_descriptor.h:
http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/ts_descriptor.h@43
PS25, Line 43: class HostPort;
> No longer needed?
Done
--
To view, visit http://gerrit.cloudera.org:8080/14177
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 25
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 19 Mar 2020 23:15:27 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2780: create thread for auto-rebalancing
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14177 to look at the new patch set (#26). Change subject: KUDU-2780: create thread for auto-rebalancing .. KUDU-2780: create thread for auto-rebalancing The auto-rebalancer thread is a background task of the master's catalog manager. Each loop iteration, the thread collects information on tablet servers, tables, and tablets, in order to determine the best rebalancing replica moves for the current state of the cluster. The maximum number of moves per tserver, per iteration, is controlled by a flag. Currently, each batch of moves scheduled per iteration is executed synchronously. The thread waits for the moves to complete, then sleeps for a time interval, which is controlled by a flag, before resuming. If the cluster has placement policy violations, the thread will prioritize scheduling and executing replica moves to reinstate the policy. Otherwise, the thread will perform inter-location (cross-location), then intra-location (by table, then by tserver) rebalancing. By default, auto-rebalancing is disabled. This can be changed by a flag in the catalog manager. The rebalancer tool should be run first, to fully rebalance the cluster, before enabling auto-rebalancing. Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/auto_rebalancer-test.cc A src/kudu/master/auto_rebalancer.cc A src/kudu/master/auto_rebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master_runner.cc M src/kudu/master/ts_descriptor.h M src/kudu/master/ts_manager.cc M src/kudu/master/ts_manager.h M src/kudu/rebalance/rebalancer.cc M src/kudu/rebalance/rebalancer.h M src/kudu/tools/rebalancer_tool.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 15 files changed, 1,756 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/14177/26 -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 26 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR](branch-1.12.x) WIP [docs] Kudu 1.12 release notes draft
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/15685 ) Change subject: WIP [docs] Kudu 1.12 release notes draft .. Patch Set 2: (1 comment) Drafted a release note for auto-rebalancing! http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc File docs/release_notes.adoc: http://gerrit.cloudera.org:8080/#/c/15685/2/docs/release_notes.adoc@63 PS2, Line 63: TODO: automatic tablet rebalancing Kudu can automatically rebalance tablet replicas among tablet servers. The background task can be enabled by setting the `auto_rebalancing_enabled` flag in the catalog manager. Before starting auto-rebalancing on an existing cluster, the CLI rebalancer tool should be run first (see link:https://issues.apache.org/jira/browse/KUDU-2780[KUDU-2780]). -- To view, visit http://gerrit.cloudera.org:8080/15685 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.12.x Gerrit-MessageType: comment Gerrit-Change-Id: I300fb597a4eed36199ebf8760084e6df1fb04e9a Gerrit-Change-Number: 15685 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Wed, 08 Apr 2020 20:06:28 + Gerrit-HasComments: Yes
[kudu-CR] Modify table scan tool to collect and surface errors instead of crashing
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#2). Change subject: Modify table scan tool to collect and surface errors instead of crashing .. Modify table scan tool to collect and surface errors instead of crashing Initialized a Status for each thread in StartWork(). Replaced CHECKs in ScanData() with return of bad Status. ScanTask() or CopyTask() catch bad Statuses from ScanData() in out-parameter back to StartWork(). StartWork() logs and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 61 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/2 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan tool to surface errors instead of crashing
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#3). Change subject: KUDU-2851: modify table scan tool to surface errors instead of crashing .. KUDU-2851: modify table scan tool to surface errors instead of crashing - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 91 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/3 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 3 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan tool to surface errors instead of crashing
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#4). Change subject: KUDU-2851: modify table scan tool to surface errors instead of crashing .. KUDU-2851: modify table scan tool to surface errors instead of crashing - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 95 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/4 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 4 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#5). Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. KUDU-2851: modify table scan and copy tools to surface errors - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 95 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/5 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 5 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13733 )
Change subject: KUDU-2851: modify table scan and copy tools to surface errors
..
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13733/3//COMMIT_MSG
Commit Message:
PS3:
> So it'll be clear when looking back through old commits, could you also men
Done
http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/kudu-tool-test.cc@5028
PS3, Line 5028: ser
> nit: Can you remove the extra whitespace here? Same below, and above in pat
Done
http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/kudu-tool-test.cc@5035
PS3, Line 5035: enfo
> nit: this is the copy tool :)
Done
http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/table_scanner.cc@628
PS3, Line 628:
: sw.stop();
: if (out_) {
: *out_ << "Total count " << total_count_.Load()
: << " cost " << sw.elapsed().wall_seconds() << " seconds"
<< endl;
: }
:
> Looking at TableScanner's out_ a bit, it's possible that `out_` never gets
Done
--
To view, visit http://gerrit.cloudera.org:8080/13733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100
Gerrit-Change-Number: 13733
Gerrit-PatchSet: 5
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 27 Jun 2019 00:18:32 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13733 ) Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/13733/3//COMMIT_MSG Commit Message: PS3: > Done I thought I changed this in PS5? http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13733/3/src/kudu/tools/kudu-tool-test.cc@5035 PS3, Line 5035: enfo > Done Also thought this was changed in PS5? -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 5 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 27 Jun 2019 00:26:28 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#6). Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. KUDU-2851: modify table scan and copy tools to surface errors - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 95 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/6 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 6 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#7). Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. KUDU-2851: modify table scan and copy tools to surface errors - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 94 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/7 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 7 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13733 )
Change subject: KUDU-2851: modify table scan and copy tools to surface errors
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/table_scanner.cc@468
PS7, Line 468:
: Status scan_status = Status::OK();
:
: for (auto token : tokens) {
: Stopwatch sw(Stopwatch::THIS_THREAD);
: sw.start();
:
: KuduScanner* scanner_ptr;
: scan_status = token->IntoKuduScanner(&scanner_ptr);
: RETURN_NOT_OK(scan_status);
:
: unique_ptr scanner(scanner_ptr);
: scan_status = scanner->Open();
: RETURN_NOT_OK(scan_status);
:
: uint64_t count = 0;
: while (scanner->HasMoreRows()) {
: KuduScanBatch batch;
: scan_status = scanner->NextBatch(&batch);
: RETURN_NOT_OK(scan_status);
: count += batch.NumRows();
: total_count_.IncrementBy(batch.NumRows());
: cb(batch);
: }
:
: sw.stop();
: if (out_) {
: MutexLock l(output_lock_);
: *out_ << "T " << token->tablet().id() << " scanned count "
<< count
:<< " cost " << sw.elapsed().wall_seconds() << "
seconds" << endl;
: }
: }
:
: return scan_status;
> A quick browse on this file, and I suggest to give up the local variable 's
Done
--
To view, visit http://gerrit.cloudera.org:8080/13733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100
Gerrit-Change-Number: 13733
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Thu, 27 Jun 2019 02:29:28 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13733 )
Change subject: KUDU-2851: modify table scan and copy tools to surface errors
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/kudu-tool-test.cc@5030
PS7, Line 5030: TEST_F(ToolTest, TestFailedTableScan) {
> error: static data member 'test_info_' not allowed in local class 'ToolTest
Done
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/kudu-tool-test.cc@5056
PS7, Line 5056: TEST_F(ToolTest, TestFailedTableCopy) {
> error: function definition is not allowed here [clang-diagnostic-error]
Done
http://gerrit.cloudera.org:8080/#/c/13733/7/src/kudu/tools/kudu-tool-test.cc@5088
PS7, Line 5088: } // namespace kudu
> error: expected '}' [clang-diagnostic-error]
Done
--
To view, visit http://gerrit.cloudera.org:8080/13733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100
Gerrit-Change-Number: 13733
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Thu, 27 Jun 2019 02:30:02 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, helifu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#8). Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. KUDU-2851: modify table scan and copy tools to surface errors - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 91 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/8 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 8 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2851: modify table scan and copy tools to surface errors
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, helifu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13733 to look at the new patch set (#9). Change subject: KUDU-2851: modify table scan and copy tools to surface errors .. KUDU-2851: modify table scan and copy tools to surface errors - Initialized a Status for each thread. - Replaced CHECKs in ScanData() with return of bad Status. - StartWork() logs all bad Statuses, and returns the first bad Status from the thread pool. Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/table_scanner.cc M src/kudu/tools/table_scanner.h 3 files changed, 87 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13733/9 -- To view, visit http://gerrit.cloudera.org:8080/13733 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic45da537b8bacfa9625010536ea82da9a6e76100 Gerrit-Change-Number: 13733 Gerrit-PatchSet: 9 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2780: create thread for auto-rebalancing (1/n)
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13891 Change subject: KUDU-2780: create thread for auto-rebalancing (1/n) .. KUDU-2780: create thread for auto-rebalancing (1/n) AutoRebalancerTask is a thread in the master with a configurable polling interval. Every interval, the thread logs the current skew of the cluster if auto-rebalancing is enabled. Code that is modified/copied directly from the CLI rebalancing tool in kudu/tools is marked. The task currently calls the 'ksck' tool to determine skew, but this may change in later patches when rebalancing is implemented and the task retrieves information directly from the master. Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c --- M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/autorebalancer-test.cc A src/kudu/master/autorebalancer.cc A src/kudu/master/autorebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 7 files changed, 1,769 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13891/1 -- To view, visit http://gerrit.cloudera.org:8080/13891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c Gerrit-Change-Number: 13891 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen
[kudu-CR] KUDU-2780: create thread for auto-rebalancing (1/n)
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13891 )
Change subject: KUDU-2780: create thread for auto-rebalancing (1/n)
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13891/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13891/1//COMMIT_MSG@13
PS1, Line 13: Code that is modified/copied directly from the CLI rebalancing
: tool in kudu/tools is marked.
> Did you consider separating the rebalancing piece that doesn't depend on th
Hmmm, the point of this patch was mostly to lay the infrastructure (create the
Task in the master) by implementing some code that would log the current skew
of the cluster.
I don't think this patch will be merge-able, but the next one (which will
migrate any common code into the master directory, and retrieve information
from the master and not call 'ksck' at all) should be.
Will clarify this in the message.
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/CMakeLists.txt@61
PS1, Line 61: ksck
> This introduce a cycling dependency which is a no-no:
Will refactor the auto-rebalancer to not call 'ksck' at all.
Then moving common code into the master should also be ok, since tools already
depends on the master directory.
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/autorebalancer-test.cc
File src/kudu/master/autorebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/autorebalancer-test.cc@55
PS1, Line 55: TEST_F(AutoRebalancerTest, NoReports) {
> We have many tests which start master and does some work before shutting it
Agreed, this test isn't necessary.
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/autorebalancer-test.cc@69
PS1, Line 69: TEST_F(AutoRebalancerTest, ViewReports) {
> What does this test tries to verify?
This test keeps a cluster around for at least a couple polling intervals, and
since the auto-rebalancer is currently just logging the skew of the cluster,
running the test should print some cluster reports of skew.
I just looked at the output on my terminal manually. Is there a better way to
write the test to check that it appears?
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/autorebalancer.h
File src/kudu/master/autorebalancer.h:
http://gerrit.cloudera.org:8080/#/c/13891/1/src/kudu/master/autorebalancer.h@74
PS1, Line 74:
> nit: we use spaces, not tabs in Kudu C++ for indentation.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13891
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c
Gerrit-Change-Number: 13891
Gerrit-PatchSet: 1
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 23 Jul 2019 01:16:07 +
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing (1/n)
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13891 to look at the new patch set (#2). Change subject: WIP KUDU-2780: create thread for auto-rebalancing (1/n) .. WIP KUDU-2780: create thread for auto-rebalancing (1/n) AutoRebalancerTask is a thread in the master with a configurable polling interval. Every interval, the thread logs the current skew of the cluster if auto-rebalancing is enabled. Code that is modified/copied directly from the CLI rebalancing tool in kudu/tools is marked. The task currently calls the 'ksck' tool to determine skew, but this will change in later patches when rebalancing is implemented and the task retrieves information directly from the master. Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c --- M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/autorebalancer-test.cc A src/kudu/master/autorebalancer.cc A src/kudu/master/autorebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 7 files changed, 1,753 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13891/2 -- To view, visit http://gerrit.cloudera.org:8080/13891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c Gerrit-Change-Number: 13891 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing (1/n)
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13891 to look at the new patch set (#3). Change subject: WIP KUDU-2780: create thread for auto-rebalancing (1/n) .. WIP KUDU-2780: create thread for auto-rebalancing (1/n) AutoRebalancerTask is a thread in the master with a configurable polling interval. Every interval, the thread logs the current skew of the cluster if auto-rebalancing is enabled. Code that is modified/copied directly from the CLI rebalancing tool in kudu/tools is marked. The task currently calls the 'ksck' tool to determine skew, but this will change in later patches when rebalancing is implemented and the task retrieves information directly from the master. Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c --- M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/autorebalancer-test.cc A src/kudu/master/autorebalancer.cc A src/kudu/master/autorebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 7 files changed, 1,753 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13891/3 -- To view, visit http://gerrit.cloudera.org:8080/13891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c Gerrit-Change-Number: 13891 Gerrit-PatchSet: 3 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing (1/n)
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13891 to look at the new patch set (#4). Change subject: WIP KUDU-2780: create thread for auto-rebalancing (1/n) .. WIP KUDU-2780: create thread for auto-rebalancing (1/n) AutoRebalancerTask is a thread in the master with a configurable polling interval. Every interval, the thread logs the current skew of the cluster if auto-rebalancing is enabled. Code that is modified/copied directly from the CLI rebalancing tool in kudu/tools is marked. The task currently calls the 'ksck' tool to determine skew, but this will change in later patches when rebalancing is implemented and the task retrieves information directly from the master. Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c --- M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/autorebalancer-test.cc A src/kudu/master/autorebalancer.cc A src/kudu/master/autorebalancer.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 7 files changed, 1,753 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/13891/4 -- To view, visit http://gerrit.cloudera.org:8080/13891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c Gerrit-Change-Number: 13891 Gerrit-PatchSet: 4 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Minor update to kudu-tool-test for macOS.
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14085 Change subject: Minor update to kudu-tool-test for macOS. .. Minor update to kudu-tool-test for macOS. Test ClusterNameResolverFileCorrupt should fail with a slightly different error message when in macOS instead of Linux. Change-Id: I6ccf7103ae59fe93087be8654cd663a043d2be0e --- M src/kudu/tools/kudu-tool-test.cc 1 file changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/14085/1 -- To view, visit http://gerrit.cloudera.org:8080/14085 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6ccf7103ae59fe93087be8654cd663a043d2be0e Gerrit-Change-Number: 14085 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen
[kudu-CR] WIP KUDU-2780: create thread for auto-rebalancing (1/n)
Hannah Nguyen has abandoned this change. ( http://gerrit.cloudera.org:8080/13891 ) Change subject: WIP KUDU-2780: create thread for auto-rebalancing (1/n) .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/13891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ibc829d14dc8636287bf3a4faa4ba76e86ab6885c Gerrit-Change-Number: 13891 Gerrit-PatchSet: 4 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Minor update to kudu-tool-test for macOS.
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14085 )
Change subject: Minor update to kudu-tool-test for macOS.
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14085/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14085/1/src/kudu/tools/kudu-tool-test.cc@627
PS1, Line 627: }
> warning: the 'empty' method should be used to check for emptiness instead o
Done
http://gerrit.cloudera.org:8080/#/c/14085/1/src/kudu/tools/kudu-tool-test.cc@4922
PS1, Line 4922: "address for bad: Name or service not
known")));
: #endif
: }
:
: TEST_F(ToolTest, ClusterNameResolverNormal) {
: ExternalMiniClusterOptions opts;
> Rather than bake the possibility of up to two errors into the code, use the
Done
http://gerrit.cloudera.org:8080/#/c/14085/1/src/kudu/tools/kudu-tool-test.cc@4926
PS1, Line 4926: meResolverNormal) {
: ExternalMiniClusterOptions opts;
> maybe, just match the error message with shorter string
Done
--
To view, visit http://gerrit.cloudera.org:8080/14085
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ccf7103ae59fe93087be8654cd663a043d2be0e
Gerrit-Change-Number: 14085
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Hannah Nguyen
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 16 Aug 2019 23:55:34 +
Gerrit-HasComments: Yes
[kudu-CR] Minor update to kudu-tool-test for macOS.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14085 to look at the new patch set (#2). Change subject: Minor update to kudu-tool-test for macOS. .. Minor update to kudu-tool-test for macOS. Test ClusterNameResolverFileCorrupt should fail with a slightly different error message when in macOS instead of Linux. Change-Id: I6ccf7103ae59fe93087be8654cd663a043d2be0e --- M src/kudu/tools/kudu-tool-test.cc 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/14085/2 -- To view, visit http://gerrit.cloudera.org:8080/14085 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ccf7103ae59fe93087be8654cd663a043d2be0e Gerrit-Change-Number: 14085 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Move some rebalancing logic from tools to master
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14110 Change subject: Move some rebalancing logic from tools to master .. Move some rebalancing logic from tools to master Moved rebalance_algo.h, rebalance_algo.cc, and rebalance_algo-test.cc from tools into master. Pulled structures from tools/ksck_results into master/cluster_status. TODO: move tools/placement_policy_util and tools/rebalancer into master too, so future auto-rebalancer task does not rely on tools. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/cluster_status.cc A src/kudu/master/cluster_status.h R src/kudu/master/rebalance_algo-test.cc R src/kudu/master/rebalance_algo.cc R src/kudu/master/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 22 files changed, 782 insertions(+), 588 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/1 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen
[kudu-CR] Move some rebalancing logic from tools to master
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: Move some rebalancing logic from tools to master .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc File src/kudu/tools/ksck_results.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@180 PS1, Line 180: (preemptive) TODO: fix the errant whitespaces in this file -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 21 Aug 2019 01:16:29 + Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: Move some rebalancing logic from tools to master .. Patch Set 1: (14 comments) http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/master/rebalance_algo.cc File src/kudu/master/rebalance_algo.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/master/rebalance_algo.cc@198 PS1, Line 198: random_device_(), > warning: initializer for member 'random_device_' is redundant [readability- Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc@67 PS1, Line 67: using kudu::master::ClusterCheckResult; > warning: using decl 'ClusterCheckResult' is unused [misc-unused-using-decls Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc@542 PS1, Line 542:const string& ref_id, > warning: parameter 'ref_id' is unused [misc-unused-parameters] Took out parameter in function signature and in function calls. http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc File src/kudu/tools/ksck_results.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@61 PS1, Line 61: using kudu::master::ClusterStatus; > warning: using decl 'ClusterStatus' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@180 PS1, Line 180: > (preemptive) TODO: fix the errant whitespaces in this file Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/placement_policy_util.cc File src/kudu/tools/placement_policy_util.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/placement_policy_util.cc@47 PS1, Line 47: //TODO: remove > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc File src/kudu/tools/rebalance-test.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@32 PS1, Line 32: #include "kudu/tools/ksck_results.h" //TODO: remove > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@33 PS1, Line 33: #include "kudu/tools/rebalancer.h" //TODO: remove > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@46 PS1, Line 46: using kudu::master::ClusterLocalityInfo; > warning: using decl 'ClusterLocalityInfo' is unused [misc-unused-using-decl Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@456 PS1, Line 456: //TODO: put this back in before the operator<< > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc File src/kudu/tools/rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@57 PS1, Line 57: using kudu::client::KuduClient; > warning: using decl 'KuduClient' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@65 PS1, Line 65: using kudu::master::LocationBalancingAlgo; > warning: using decl 'LocationBalancingAlgo' is unused [misc-unused-using-de Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@66 PS1, Line 66: using kudu::master::RebalancingAlgo; > warning: using decl 'RebalancingAlgo' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@69 PS1, Line 69: using kudu::master::TwoDimensionalGreedyAlgo; > warning: using decl 'TwoDimensionalGreedyAlgo' is unused [misc-unused-using Done -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 1 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 21 Aug 2019 15:28:52 + Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#2). Change subject: Move some rebalancing logic from tools to master .. Move some rebalancing logic from tools to master Moved rebalance_algo.h, rebalance_algo.cc, and rebalance_algo-test.cc from tools into master. Pulled structures from tools/ksck_results into master/cluster_status. TODO: move tools/placement_policy_util and tools/rebalancer into master too, so future auto-rebalancer task does not rely on tools. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/cluster_status.cc A src/kudu/master/cluster_status.h R src/kudu/master/rebalance_algo-test.cc R src/kudu/master/rebalance_algo.cc R src/kudu/master/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 22 files changed, 779 insertions(+), 594 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/2 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Move some rebalancing logic from tools to master
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: Move some rebalancing logic from tools to master .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15 PS2, Line 15: TODO: move tools/placement_policy_util and tools/rebalancer : into master too, so future auto-rebalancer task does not : rely on tools. > Yep, it makes sense to do so. Probably, it's best to do so in this changel Do you think it should be done in parts? ie. pt1 and pt2? I was concerned the patch would be too big if I submitted it all at once. http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt File src/kudu/master/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48 PS2, Line 48: rebalance_algo.cc > Agreed with Alexey. The rebalancing policy stuff is enough code that it pro Do you mean adding another directory like kudu/rebalance and moving all this code there? Or leaving this code in kudu/master but not in the master namespace, adding a target and library to kudu/master's CMakelists.txt? -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 2 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 21 Aug 2019 18:10:06 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#3). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 845 insertions(+), 654 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/3 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 3 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
..
Patch Set 3:
(12 comments)
I've uploaded PS3 as a WIP patch to address everything here so far, but will
put up the remaining code to move in PS4.
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15
PS2, Line 15: Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
:
:
> If the patch is about moving things around and renaming, it should not be h
No, just wanted to make sure it was review-able.
I will add the other changes (tools/placement_policy_util and tools/rebalancer)
into PS4.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: sentry_client_met
> Eventually, only master is going to use that code (and the kudu CLI will si
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h
File src/kudu/master/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@33
PS2, Line 33:
> nit: I'm not sure that 'master' is the best namespace to host cluster-relat
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@79
PS2, Line 79:
:
:
:
:
> nit: given the renames, be sure to fix any spacing issues this may have cau
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@119
PS2, Line 119:
> nit: remove and maybe just mention the rename in the commit message. Same w
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc
File src/kudu/master/cluster_status.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc@72
PS2, Line 72:
:
:
:
:
:
:
:
:
:
:
:
> I know this just came from the prior revision, but I'm not sure this is nee
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc@67
PS2, Line 67: using kudu::rebalance::ClusterReplicaSummary;
: using kudu::rebalance::ClusterServerHealth;
: using kudu::rebalance::ClusterServerHealthSummary;
: using kudu::rebalance::ClusterTableSummary;
: using kudu::rebalance::ClusterTabletSummary;
: using kudu::server::GetFlagsResponsePB;
: using kudu::tablet::TabletDataState;
:
: using std::ostringstream;
: using std::shared_ptr;
: using std::static_pointer_cast;
: using std::string;
: using std::unordered_map;
: using std::vector;
: using strings::Substitute;
:
: namespace kudu {
: namespace tools {
:
> nit: you can drop the 'kudu::' prefix, or move all of these out of the name
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@114
PS2, Line 114: // Collection of error status for failed checks. Used to print
out a final
: // summary of all failed checks.
: // All checks passed if and only if this vector is empty.
: std::vector error_messages;
:
: // Collection of warnings from checks.
: // These errors are
> nit: remove this?
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@190
PS2, Line 190: // summary information will be printed, while in PLAIN_FULL
mode, the counts for
> warning: function 'kudu::tools::PrintConsensusMatrix' has a definition with
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@191
PS2, Line 191: // every tablet server will be printed as well.
> warning: parameter 'ref_cstate' is const-qualified in the function declarat
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc@127
PS2, Line 127: DataTable* table) {
> warning: the const qualified parameter 'replica_labels' is copied for each
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc@307
PS2, Line 307: const ClusterConsensusStateMap&
cstates,
> warning: the const qualified parameter 'ref_cstate' is copied for each invo
Done
--
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 ) Change subject: WIP: Create kudu/rebalance subdirectory .. Patch Set 3: > (12 comments) > > I've uploaded PS3 as a WIP patch to address everything here so far, > but will put up the remaining code to move in this patch later. -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 3 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 22 Aug 2019 01:09:59 + Gerrit-HasComments: No
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14110 to look at the new patch set (#4). Change subject: WIP: Create kudu/rebalance subdirectory .. WIP: Create kudu/rebalance subdirectory Moved structs used for rebalancing from tools/ksck_results into rebalance/cluster_status. Moved contents of tools/rebalance_algo into rebalance subdirectory too. TODO: move tools/placement_policy_util and tools/rebalancer. Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 --- M CMakeLists.txt A src/kudu/rebalance/CMakeLists.txt A src/kudu/rebalance/cluster_status.cc A src/kudu/rebalance/cluster_status.h R src/kudu/rebalance/rebalance_algo-test.cc R src/kudu/rebalance/rebalance_algo.cc R src/kudu/rebalance/rebalance_algo.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_remote.cc M src/kudu/tools/ksck_remote.h M src/kudu/tools/ksck_results.cc M src/kudu/tools/ksck_results.h M src/kudu/tools/placement_policy_util-test.cc M src/kudu/tools/placement_policy_util.cc M src/kudu/tools/placement_policy_util.h M src/kudu/tools/rebalance-test.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/rebalancer.h M src/kudu/tools/tool.proto M src/kudu/tools/tool_action_cluster.cc 23 files changed, 859 insertions(+), 658 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/4 -- To view, visit http://gerrit.cloudera.org:8080/14110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964 Gerrit-Change-Number: 14110 Gerrit-PatchSet: 4 Gerrit-Owner: Hannah Nguyen Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hannah Nguyen Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
