acelyc111 commented on code in PR #1006: URL: https://github.com/apache/incubator-pegasus/pull/1006#discussion_r902129328
########## admin-cli/cmd/table_migrator.go: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cmd + +import ( + "github.com/apache/incubator-pegasus/admin-cli/executor/toolkits/tablemigrator" + "github.com/apache/incubator-pegasus/admin-cli/shell" + "github.com/desertbit/grumble" +) + +func init() { + shell.AddCommand(&grumble.Command{ + Name: "table-migrator", + Help: "migrate table from current cluster to another via table duplication and metaproxy", + Flags: func(f *grumble.Flags) { + f.String("t", "table", "", "table name") + f.String("n", "node", "", "zk node, addrs:port, default equal with peagsus "+ + "cluster zk addrs, you can use `cluster-info` to show it") + f.String("r", "root", "", "zk root path. the tool will update table addrs in "+ + "the path of meatproxy, if you don't specify it, that is means user need manual-switch the table addrs") + f.String("c", "cluster", "", "target cluster name") + f.String("m", "meta", "", "target meta list") + f.Float64("p", "threshold", 100000, "pending mutation throshold when server will reject all write request") Review Comment: ```suggestion f.Float64("p", "threshold", 100000, "pending mutation threshold when server will reject all write request") ``` ```suggestion f.Float64("p", "threshold", 100000, "pending mutation throshold when server will reject all write request") ``` From the description, I still don't know what's it mean, and what's it effect. Could you plz give more description about it? ########## admin-cli/cmd/table_migrator.go: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cmd + +import ( + "github.com/apache/incubator-pegasus/admin-cli/executor/toolkits/tablemigrator" + "github.com/apache/incubator-pegasus/admin-cli/shell" + "github.com/desertbit/grumble" +) + +func init() { + shell.AddCommand(&grumble.Command{ + Name: "table-migrator", + Help: "migrate table from current cluster to another via table duplication and metaproxy", Review Comment: Is the migrate tool bind to metaproxy? AFAIK, metaproxy is used for unify table access, how can it effect the migrate tool? ########## admin-cli/cmd/table_version.go: ########## @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cmd + +import ( + "github.com/apache/incubator-pegasus/admin-cli/executor" + "github.com/apache/incubator-pegasus/admin-cli/shell" + "github.com/desertbit/grumble" +) + +func init() { + shell.AddCommand(&grumble.Command{ + Name: "data-version", Review Comment: what's data version? ########## admin-cli/executor/server_config.go: ########## @@ -20,26 +20,20 @@ package executor Review Comment: Is this file modification related to this patch? ########## admin-cli/cmd/table_migrator.go: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cmd + +import ( + "github.com/apache/incubator-pegasus/admin-cli/executor/toolkits/tablemigrator" + "github.com/apache/incubator-pegasus/admin-cli/shell" + "github.com/desertbit/grumble" +) + +func init() { + shell.AddCommand(&grumble.Command{ + Name: "table-migrator", + Help: "migrate table from current cluster to another via table duplication and metaproxy", + Flags: func(f *grumble.Flags) { + f.String("t", "table", "", "table name") + f.String("n", "node", "", "zk node, addrs:port, default equal with peagsus "+ + "cluster zk addrs, you can use `cluster-info` to show it") + f.String("r", "root", "", "zk root path. the tool will update table addrs in "+ + "the path of meatproxy, if you don't specify it, that is means user need manual-switch the table addrs") + f.String("c", "cluster", "", "target cluster name") Review Comment: is 'cluster name' used for duplication? Could you describe how to define and use it? ########## admin-cli/executor/table_version.go: ########## @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package executor + +import ( + "encoding/json" + "fmt" + "strconv" + + "github.com/apache/incubator-pegasus/admin-cli/util" + "github.com/apache/incubator-pegasus/go-client/session" +) + +type TableDataVersion struct { + DataVersion string `json:"data_version"` Review Comment: I think it would be necessary to describe what is data vesion firstly. :) ########## admin-cli/util/http_client.go: ########## @@ -0,0 +1,78 @@ +/* Review Comment: Seems not highly related to this patch, would it be better to create another pr for it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
