Skip to content

[IOTDB-188] Delete storage group#416

Merged
qiaojialin merged 7 commits intoapache:masterfrom
nlosilva1:delStorageGroup
Sep 26, 2019
Merged

[IOTDB-188] Delete storage group#416
qiaojialin merged 7 commits intoapache:masterfrom
nlosilva1:delStorageGroup

Conversation

@nlosilva1
Copy link
Contributor

@nlosilva1 nlosilva1 commented Sep 24, 2019

Complete a function using session to delete storage groups in iotdb. Delete all the data of this storage group after calling this function.

Jira link: https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-188

return true;
}

public void rmStorageGroupInfo(String storageGroupName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to deleteStorageGroup(), call deleteAllDataFilesInOneStorageGroup() first in this method.

waitForAllCurrentTsFileProcessorsClosed();
writeLock();
try {
File storageGroupFolder = TSFileFactory.INSTANCE.getFile(systemDir, storageGroupName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be the SystemFactory

case MetadataOperationType.SET_STORAGE_LEVEL_TO_MTREE:
setStorageLevelToMTree(args[1]);
break;
case MetadataOperationType.DELETE_STORAGE_LEVEL_TO_MTREE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case MetadataOperationType.DELETE_STORAGE_LEVEL_TO_MTREE:
case MetadataOperationType.DELETE_STORAGE_GROUP_FROM_MTREE:

@@ -212,6 +212,9 @@ private void operation(String cmd)
case MetadataOperationType.SET_STORAGE_LEVEL_TO_MTREE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case MetadataOperationType.SET_STORAGE_LEVEL_TO_MTREE:
case MetadataOperationType.SET_STORAGE_GROUP_TO_MTREE:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiaojialin Do you want to modify all STORAGE_LEVEL to STORAGE_GROUP? If in that case, the author needs to check all naming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

/**
* function for deleting storage level of the given path to mTree.
*/
public boolean deleteStorageLevelToMTree(String path) throws MetadataErrorException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public boolean deleteStorageLevelToMTree(String path) throws MetadataErrorException {
public boolean deleteStorageGroupFromMTree(String path) throws MetadataErrorException {

TSStatus deleteTimeseries(1:list<string> path)
TSStatus deleteTimeseries(1:list<string> path)

TSStatus deleteStorageGroup(1:TSDeleteDataReq req);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TSStatus deleteStorageGroup(1:TSDeleteDataReq req);
TSStatus deleteStorageGroup(1:string storageGroup);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best if you make it a list.

}
}

public synchronized TSStatus deleteStorageGroup(String storageGroupId, long time) throws IoTDBSessionException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public synchronized TSStatus deleteStorageGroup(String storageGroupId, long time) throws IoTDBSessionException {
public synchronized TSStatus deleteStorageGroup(String storageGroupId) throws IoTDBSessionException {

List<String> paths = new ArrayList<>();
paths.add(storageGroupId);
request.setPaths(paths);
request.setTimestamp(time);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this 5 lines

request.setPaths(paths);
request.setTimestamp(time);
try {
return checkAndReturn(client.deleteStorageGroup(request));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return checkAndReturn(client.deleteStorageGroup(request));
return checkAndReturn(client.deleteStorageGroup(storageGroup));

}
}

public void deleteStorageGroupTest() throws ClassNotFoundException, SQLException, IoTDBSessionException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public void deleteStorageGroupTest() throws ClassNotFoundException, SQLException, IoTDBSessionException {
public void deleteStorageGroup() throws ClassNotFoundException, SQLException, IoTDBSessionException {

@@ -212,6 +212,9 @@ private void operation(String cmd)
case MetadataOperationType.SET_STORAGE_LEVEL_TO_MTREE:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qiaojialin Do you want to modify all STORAGE_LEVEL to STORAGE_GROUP? If in that case, the author needs to check all naming.

Copy link
Contributor

@jt2594838 jt2594838 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great but with some small issues.

logger.info(INFO_NOT_LOGIN, IoTDBConstant.GLOBAL_DB_NAME);
return new TSStatus(getStatus(TSStatusCode.NOT_LOGIN_ERROR));
}
MetadataPlan plan = new MetadataPlan(MetadataOperator.NamespaceType.DELETE_STORAGE_GROUP,new Path(req.getPaths().get(0)));
Copy link
Contributor

@jt2594838 jt2594838 Sep 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So only one storage group is supported at the same time? I guess it is not difficult to support deleting multiple groups with one request, try if you can.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the issue defined in IOTDB-188, I just need to delete a single storage group at the same time, and Qiaojialin said to me that before we complete the batch deletion of storage groups, we should change some of the structure of the Metadata, so he suggested me to submit this version first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can always go beyond the issue as long as it is helpful, but it is fine to leave it as future work.

TSStatus deleteTimeseries(1:list<string> path)
TSStatus deleteTimeseries(1:list<string> path)

TSStatus deleteStorageGroup(1:TSDeleteDataReq req);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best if you make it a list.

@jt2594838
Copy link
Contributor

jt2594838 commented Sep 24, 2019

Rename the title to "[IOTDB-XXX]..." and provide the Jira link, please.

@nlosilva1 nlosilva1 changed the title Delete storage group in session [IOTDB-188] Delete storage group Sep 24, 2019
@nlosilva1 nlosilva1 requested a review from jt2594838 September 24, 2019 11:14
@qiaojialin qiaojialin merged commit 59f66f6 into apache:master Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants