You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/08/19 08:13:14 UTC
[GitHub] [iotdb] CRZbulabula opened a new pull request, #7060: [IOTDB-4174] Show RegionNum when show storagegroup
CRZbulabula opened a new pull request, #7060:
URL: https://github.com/apache/iotdb/pull/7060
Recommend by @ericpai
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950775349
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -176,7 +176,7 @@ public CountStorageGroupResp countMatchedStorageGroups(CountStorageGroupPlan pla
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name" + e.getMessage()));
Review Comment:
Fixed.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] ericpai commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r949924120
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -202,7 +202,7 @@ public StorageGroupSchemaResp getMatchedStorageGroupSchemas(GetStorageGroupPlan
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name: " + e));
Review Comment:
`+ e.getMessage()`? I think `getMessage()` is more friendly for human read.
##########
confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java:
##########
@@ -568,9 +567,13 @@ public TShowConfigNodesResp showConfigNodes() throws TException {
return configManager.showConfigNodes();
}
- public void handleClientExit() {}
+ @Override
+ public TShowStorageGroupResp showStorageGroup(List<String> storageGroupPathPattern)
+ throws TException {
+ return configManager.showStorageGroup(new GetStorageGroupPlan(storageGroupPathPattern));
+ }
- // TODO: Interfaces for data operations
+ public void handleClientExit() {}
Review Comment:
What's this empty method used for?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] qiaojialin merged pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
qiaojialin merged PR #7060:
URL: https://github.com/apache/iotdb/pull/7060
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Beyyes commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
Beyyes commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950654037
##########
confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceHandler.java:
##########
@@ -47,8 +44,6 @@ public ServerContext createContext(TProtocol arg0, TProtocol arg1) {
@Override
public void deleteContext(ServerContext arg0, TProtocol arg1, TProtocol arg2) {
- // release query resources.
- processor.handleClientExit();
Review Comment:
what's the usage of `processor.handleClientExit();` before?
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -157,7 +157,7 @@ public TSStatus deleteStorageGroup(DeleteStorageGroupPlan plan) {
LOGGER.warn("Storage group not exist", e);
result
.setCode(TSStatusCode.SUCCESS_STATUS.getStatusCode())
- .setMessage("Storage group not exist");
+ .setMessage("Storage group not exist" + e.getMessage());
Review Comment:
```suggestion
.setMessage("Storage group not exist: " + e.getMessage());
```
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -176,7 +176,7 @@ public CountStorageGroupResp countMatchedStorageGroups(CountStorageGroupPlan pla
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name" + e.getMessage()));
Review Comment:
```suggestion
.setMessage("Error StorageGroup name: " + e.getMessage()));
```
##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/IManager.java:
##########
@@ -306,6 +307,9 @@ Object getOrCreateDataPartition(
/** Show ConfigNodes */
TShowConfigNodesResp showConfigNodes();
+ /** Show StorageGroup */
+ TShowStorageGroupResp showStorageGroup(GetStorageGroupPlan getStorageGroupPlan);
Review Comment:
add parameter `getStorageGroupPlan` for java doc?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950775349
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -176,7 +176,7 @@ public CountStorageGroupResp countMatchedStorageGroups(CountStorageGroupPlan pla
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name" + e.getMessage()));
Review Comment:
Fixed.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950775341
##########
confignode/src/main/java/org/apache/iotdb/confignode/manager/IManager.java:
##########
@@ -306,6 +307,9 @@ Object getOrCreateDataPartition(
/** Show ConfigNodes */
TShowConfigNodesResp showConfigNodes();
+ /** Show StorageGroup */
+ TShowStorageGroupResp showStorageGroup(GetStorageGroupPlan getStorageGroupPlan);
Review Comment:
Fixed.
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -157,7 +157,7 @@ public TSStatus deleteStorageGroup(DeleteStorageGroupPlan plan) {
LOGGER.warn("Storage group not exist", e);
result
.setCode(TSStatusCode.SUCCESS_STATUS.getStatusCode())
- .setMessage("Storage group not exist");
+ .setMessage("Storage group not exist" + e.getMessage());
Review Comment:
Fixed.
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -176,7 +176,7 @@ public CountStorageGroupResp countMatchedStorageGroups(CountStorageGroupPlan pla
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name" + e.getMessage()));
Review Comment:
Fixed.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950632754
##########
confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java:
##########
@@ -568,9 +567,13 @@ public TShowConfigNodesResp showConfigNodes() throws TException {
return configManager.showConfigNodes();
}
- public void handleClientExit() {}
+ @Override
+ public TShowStorageGroupResp showStorageGroup(List<String> storageGroupPathPattern)
+ throws TException {
+ return configManager.showStorageGroup(new GetStorageGroupPlan(storageGroupPathPattern));
+ }
- // TODO: Interfaces for data operations
+ public void handleClientExit() {}
Review Comment:
This interface was added in March and now it's useless.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7060: [IOTDB-4174] Show RegionNum when show storagegroup
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7060:
URL: https://github.com/apache/iotdb/pull/7060#discussion_r950632648
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -202,7 +202,7 @@ public StorageGroupSchemaResp getMatchedStorageGroupSchemas(GetStorageGroupPlan
LOGGER.error("Error StorageGroup name", e);
result.setStatus(
new TSStatus(TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode())
- .setMessage("Error StorageGroup name"));
+ .setMessage("Error StorageGroup name: " + e));
Review Comment:
Fixed~
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org