huizhilu commented on a change in pull request #1812:
URL: https://github.com/apache/helix/pull/1812#discussion_r665852334
##########
File path:
helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
##########
@@ -146,18 +147,29 @@ private void sendNopMessage() {
@Override
public void reset() {
- logger.info("Resetting HelixStateMachineEngine");
+ loopStateModelFactories(stateModel -> {
+ stateModel.reset();
+ String initialState =
_stateModelParser.getInitialState(stateModel.getClass());
+ stateModel.updateState(initialState);
+ }, "reset");
Review comment:
There are only 2 method names here. Enum seems too heavy as it would
need to parse the real name: `RESET("reset"), SYNC_STATE("syncState")`.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
##########
@@ -1376,12 +1384,27 @@ public void handleNewSession(String sessionId) throws
Exception {
}
}
- void handleNewSessionAsParticipant(final String sessionId) throws Exception {
+ private void handleNewSessionInManagementMode(String sessionId) throws
Exception {
+ LOG.info("Skip reset because instance is in {} status",
LiveInstance.LiveInstanceStatus.PAUSED);
+ if (!InstanceType.PARTICIPANT.equals(_instanceType)) {
Review comment:
I didn't add logic for CONTROLLER_PARTICIPANT. The logic is the same for
CONTROLLER_PARTICIPANT.
`handleNewSessionAsParticipant(sessionId);` ->
`handleNewSessionAsParticipant(sessionId, _liveInstanceInfoProvider);` I
changed it to avoid below unnecessary api.
```
handleNewSessionAsParticipant(sessionId) {
handleNewSessionAsParticipant(sessionId, _liveInstanceInfoProvider);
}
```
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java
##########
@@ -413,13 +433,16 @@ private void carryOverPreviousCurrentState() {
* remove previous current state parent nodes
*/
for (String session : sessions) {
- if (session.equals(_sessionId)) {
+ if (session.equals(sessionId)) {
continue;
}
- String path = _keyBuilder.currentStates(_instanceName,
session).getPath();
- LOG.info("Removing current states from previous sessions. path: " +
path);
- _zkclient.deleteRecursively(path);
+ PropertyKey currentStatesProperty =
keyBuilder.currentStates(instanceName, session);
+ String path = currentStatesProperty.getPath();
+ LOG.info("Removing current states from previous sessions. path: {}",
path);
+ if (!dataAccessor.removeProperty(currentStatesProperty)) {
Review comment:
Yes. Double checked. It will use recursively delete.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java
##########
@@ -162,6 +166,20 @@ public void handleNewSession() throws Exception {
setupMsgHandler();
}
+ private boolean shouldCarryOver() {
+ if (_liveInstanceInfoProvider == null) {
+ return true;
+ }
+ ZNRecord additionalLiveInstanceInfo =
_liveInstanceInfoProvider.getAdditionalLiveInstanceInfo();
Review comment:
Because each time `getAdditionalLiveInstanceInfo()` creates a ZNRecord
object. We do it like that, we'll need to call `getAdditionalLiveInstanceInfo`
twice, hence 2 ZNRecord objects.
##########
File path:
helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java
##########
@@ -1305,6 +1330,43 @@ String getMessageTarget(String resourceName, String
partitionName) {
return String.format("%s_%s", resourceName, partitionName);
}
+ private void changeParticipantStatus(String instanceName,
+ LiveInstance.LiveInstanceStatus toStatus, HelixManager manager) {
+ HelixDataAccessor accessor = manager.getHelixDataAccessor();
+ String sessionId = manager.getSessionId();
+ String path = accessor.keyBuilder().liveInstance(instanceName).getPath();
+
+ if (LiveInstance.LiveInstanceStatus.PAUSED.equals(toStatus)) {
Review comment:
I also thought about it. It could be cleaner since there are too many
if..else here. I didn't use it because I thought there are only 2 branches and
we need to do null check for `toStatus`. Let me use it for code readers :)
--
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]
