-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[IOTDB-4627]Trigger transfer #7643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
…ss of RemovingNode Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
CRZbulabula
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL~
| // do transfer of the DataNodes before remove | ||
| if (configManager.transfer(removeDataNodePlan.getDataNodeLocations()).getCode() | ||
| != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { | ||
| dataSet.setStatus( | ||
| new TSStatus(TSStatusCode.NODE_DELETE_FAILED_ERROR.getStatusCode()) | ||
| .setMessage("Fail to do transfer of the DataNodes")); | ||
| return dataSet; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the transfer process should be such important that the DataNode remove process couldn't continue as long as if it fails down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we should ensure the safety of data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to consider implement by Procedure, but later we found that we did not need to roll back when the transfer failed.
CRZbulabula
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Weihao Li <18110526956@163.com>
| * @return result of transferTrigger | ||
| */ | ||
| public TSStatus transferTrigger( | ||
| List<TDataNodeLocation> newUnknownDataList, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| List<TDataNodeLocation> newUnknownDataList, | |
| List<TDataNodeLocation> newUnknownDataNodeList, |
|
|
||
| public List<TSStatus> updateTriggerLocation( | ||
| String triggerName, | ||
| TDataNodeLocation dataNodeLocation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter dataNodeLocation and dataNodeLocationMap are similar, maybe we can change dataNodeLocation to targetDataNode?
| // do transfer of the DataNodes before remove | ||
| if (configManager.transfer(removeDataNodePlan.getDataNodeLocations()).getCode() | ||
| != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { | ||
| dataSet.setStatus( | ||
| new TSStatus(TSStatusCode.NODE_DELETE_FAILED_ERROR.getStatusCode()) | ||
| .setMessage("Fail to do transfer of the DataNodes")); | ||
| return dataSet; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

1.Transfer stateful triggers when DataNode Status is Unknown;
2.Transfer stateful triggers when DataNode remove;
3.Has been tested, can see Trigger测试方案
4.Cooperate with @lancelly.