[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)