You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sumitagrawl (via GitHub)" <gi...@apache.org> on 2024/07/11 15:25:52 UTC
[PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
sumitagrawl opened a new pull request, #6928:
URL: https://github.com/apache/ozone/pull/6928
## What changes were proposed in this pull request?
readonly mode is provided for applyTransaction request handling failure. By default, this is "disabled".
On enable the conf `ozone.om.onfailure.readonly.enable` , the feature gets enabled. And on any request failure (earlier causing crash), it moves to readonly mode, providing support for only read request.
OMStateMachine is paused(), and do not process any further request. This can be recovered only during startup when related raft log execution gets success (or skipped).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11116
## How was this patch tested?
- Unit test
- integration test covering scenario of moving to readonly mode on failure, and over restart remain in readonly mode as failed again for the raft log
Observation:
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1677801352
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -551,6 +579,24 @@ private OMResponse runCommand(OMRequest request, TermIndex termIndex) {
return createErrorResponse(request, e, termIndex);
} catch (Throwable e) {
// For any Runtime exceptions, terminate OM.
+ final boolean onFailureReadonlyEnable = ozoneManager.getConfiguration()
+ .getBoolean(OMConfigKeys.OZONE_OM_ONFAILURE_READONLY,
+ OMConfigKeys.OZONE_OM_ONFAILURE_READONLY_DEFAULT);
+ if (onFailureReadonlyEnable) {
+ LOG.error("Failed to write, moving to readonly mode, Exception occurred ", e);
+ if (readonlyMode.compareAndSet(false, true)) {
+ // update readonly mode, flush existing transaction and pause state machine
+ readonlyMode.set(true);
+ try {
+ awaitDoubleBufferFlush();
+ } catch (InterruptedException ex) {
+ Thread.interrupted();
+ }
+ pause();
+ }
+ return createErrorResponse(request, new OMException("Exception occurred and moving to readonly mode",
+ e, OMException.ResultCodes.READONLY_MODE), termIndex);
+ }
String errorMessage = "Request " + request + " failed with exception";
ExitUtils.terminate(1, errorMessage, e, LOG);
Review Comment:
@errose28 `4. `Handled for processResponse --> `INTERNAL_ERROR`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #6928:
URL: https://github.com/apache/ozone/pull/6928#issuecomment-2249326454
> > in this mode in integration test, frequent election is happening. This might be due to state machine is not getting updated with latest transaction.
@errose28
Updated description, and related to frequent leader election,
- This is not happening when test case is run
- It is observed in debug mode, when breakpoint hit and waiting for few seconds while checking parameters, as in ratis,
- leader do not receive response within a second (as configured for miniOzoneCluster)
So this seems fine and no issue with leader election.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1677645226
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -287,6 +301,11 @@ public TransactionContext preAppendTransaction(TransactionContext trx)
final OMRequest request = (OMRequest) trx.getStateMachineContext();
OzoneManagerProtocolProtos.Type cmdType = request.getCmdType();
+ if (readonlyMode.get()) {
Review Comment:
1. startTransaction() is called out of synchronized lock by ratis. On exception, it return exception to client.
2. preAppend Transaction() is called after having synchronized lock, and with success, create log entry and sync to all node in ratis ring. On Exception, it also causes dropping itself as leader as logic.
- startTransaction() check immediately return failure.
- preAppend() check is to avoid continue of already triggered request waiting on lock. This is for those request waiting for previous request to complete.
So added both places for user request.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6928:
URL: https://github.com/apache/ozone/pull/6928#issuecomment-2223283031
Please wait for clean CI run in fork before opening PR.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1677702300
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -551,6 +579,24 @@ private OMResponse runCommand(OMRequest request, TermIndex termIndex) {
return createErrorResponse(request, e, termIndex);
} catch (Throwable e) {
// For any Runtime exceptions, terminate OM.
+ final boolean onFailureReadonlyEnable = ozoneManager.getConfiguration()
+ .getBoolean(OMConfigKeys.OZONE_OM_ONFAILURE_READONLY,
+ OMConfigKeys.OZONE_OM_ONFAILURE_READONLY_DEFAULT);
+ if (onFailureReadonlyEnable) {
+ LOG.error("Failed to write, moving to readonly mode, Exception occurred ", e);
+ if (readonlyMode.compareAndSet(false, true)) {
+ // update readonly mode, flush existing transaction and pause state machine
+ readonlyMode.set(true);
+ try {
+ awaitDoubleBufferFlush();
+ } catch (InterruptedException ex) {
+ Thread.interrupted();
+ }
+ pause();
+ }
+ return createErrorResponse(request, new OMException("Exception occurred and moving to readonly mode",
+ e, OMException.ResultCodes.READONLY_MODE), termIndex);
+ }
String errorMessage = "Request " + request + " failed with exception";
ExitUtils.terminate(1, errorMessage, e, LOG);
Review Comment:
4 places, terminate is used,
1. DoubleBuffer->canFlush() -- terminate on interrupt Exception -- I think we should not terminate as its normal pause() operation call supported. This is added with "HDDS-7279. Snapshot Create requires Double Buffer Flush thread to split the commit batch. ". Created [HDDS-11182](https://issues.apache.org/jira/browse/HDDS-11182) to handle same.
2. DoubleBuffer->addToBatch IOException --- This is possible due to user implementation of Response. This will be further candidate to have readonly mode but `currently not in scope `of this.
3. DoubleBuffer --> flush db fails, IMO should terminate OM as there may be possibility of disk error, and there can be switchover and new leader can take up job.
4. OMStateMachine --> processResponse --> trigger terminate on specific error like metadata error or internal error. Terminate should be fine for metadata error (for rocksdb) so that new leader can take up job. But `INTERNAL_ERROR` will not be candidate for this. This part will fix.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1677702300
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -551,6 +579,24 @@ private OMResponse runCommand(OMRequest request, TermIndex termIndex) {
return createErrorResponse(request, e, termIndex);
} catch (Throwable e) {
// For any Runtime exceptions, terminate OM.
+ final boolean onFailureReadonlyEnable = ozoneManager.getConfiguration()
+ .getBoolean(OMConfigKeys.OZONE_OM_ONFAILURE_READONLY,
+ OMConfigKeys.OZONE_OM_ONFAILURE_READONLY_DEFAULT);
+ if (onFailureReadonlyEnable) {
+ LOG.error("Failed to write, moving to readonly mode, Exception occurred ", e);
+ if (readonlyMode.compareAndSet(false, true)) {
+ // update readonly mode, flush existing transaction and pause state machine
+ readonlyMode.set(true);
+ try {
+ awaitDoubleBufferFlush();
+ } catch (InterruptedException ex) {
+ Thread.interrupted();
+ }
+ pause();
+ }
+ return createErrorResponse(request, new OMException("Exception occurred and moving to readonly mode",
+ e, OMException.ResultCodes.READONLY_MODE), termIndex);
+ }
String errorMessage = "Request " + request + " failed with exception";
ExitUtils.terminate(1, errorMessage, e, LOG);
Review Comment:
4 places, terminate is used,
1. DoubleBuffer->canFlush() -- terminate on interrupt Exception --It will terminate only case when interrupt is received when running, this is not a normal scenario, and can happen when OM is shutdown
2. DoubleBuffer->addToBatch IOException --- This is possible due to user implementation of Response. This will be further candidate to have readonly mode but `currently not in scope `of this.
3. DoubleBuffer --> flush db fails, IMO should terminate OM as there may be possibility of disk error, and there can be switchover and new leader can take up job.
4. OMStateMachine --> processResponse --> trigger terminate on specific error like metadata error or internal error. Terminate should be fine for metadata error (for rocksdb) so that new leader can take up job. But `INTERNAL_ERROR` will not be candidate for this. This part will fix.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #6928:
URL: https://github.com/apache/ozone/pull/6928#issuecomment-2245500119
@sumitagrawl can you clarify in the PR description what types of errors will put the OM in this state? I think the OM should only enter this state for errors that happen during `validateAndUpdateCache`, not for errors writing to the DB. DB errors should probably still be fatal since that is not caused by an Ozone bug.
> in this mode in integration test, frequent election is happening. This might be due to state machine is not getting updated with latest transaction.
This needs to be investigated more. It's likely that the PR missed [this step](https://github.com/apache/ozone/blob/4e9dc2faae50a48c9c9e6fc8aa1806d74c76f512/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java#L322) which is done as part of prepare.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #6928:
URL: https://github.com/apache/ozone/pull/6928#issuecomment-2287874062
Closing this PR, as readonly mode will not provide much help towards business, as job always involves read and write.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl closed pull request #6928: HDDS-11116. OM state machine move to readonly mode on failure
URL: https://github.com/apache/ozone/pull/6928
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1674611595
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -4415,4 +4415,12 @@
maximum number of buckets across all volumes.
</description>
</property>
+ <property>
+ <name>ozone.om.onfailure.readonly.enable</name>
Review Comment:
Maybe `ozone.om.readonly.on.failure` would be more intuitive?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -551,6 +579,24 @@ private OMResponse runCommand(OMRequest request, TermIndex termIndex) {
return createErrorResponse(request, e, termIndex);
} catch (Throwable e) {
// For any Runtime exceptions, terminate OM.
+ final boolean onFailureReadonlyEnable = ozoneManager.getConfiguration()
+ .getBoolean(OMConfigKeys.OZONE_OM_ONFAILURE_READONLY,
+ OMConfigKeys.OZONE_OM_ONFAILURE_READONLY_DEFAULT);
+ if (onFailureReadonlyEnable) {
+ LOG.error("Failed to write, moving to readonly mode, Exception occurred ", e);
+ if (readonlyMode.compareAndSet(false, true)) {
+ // update readonly mode, flush existing transaction and pause state machine
+ readonlyMode.set(true);
+ try {
+ awaitDoubleBufferFlush();
+ } catch (InterruptedException ex) {
+ Thread.interrupted();
+ }
+ pause();
+ }
+ return createErrorResponse(request, new OMException("Exception occurred and moving to readonly mode",
+ e, OMException.ResultCodes.READONLY_MODE), termIndex);
+ }
String errorMessage = "Request " + request + " failed with exception";
ExitUtils.terminate(1, errorMessage, e, LOG);
Review Comment:
There's actually multiple termination points in this class. I haven't looked through it in detail but we want to make sure we are going to readonly only when there is an exception processing the `validateAndUpdateCache` method. I'm not sure if we should remain in readonly mode if there is an error writing to the DB or continue to terminate in this case.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -4415,4 +4415,12 @@
maximum number of buckets across all volumes.
</description>
</property>
+ <property>
+ <name>ozone.om.onfailure.readonly.enable</name>
+ <value>false</value>
+ <tag>OZONE, OM</tag>
+ <description>
+ enable readonly mode when om state machine failure is observed.
Review Comment:
```suggestion
Put the Ozone Manager in readonly mode instead of shutting down when there is a fatal error while handling a write request.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -551,6 +579,24 @@ private OMResponse runCommand(OMRequest request, TermIndex termIndex) {
return createErrorResponse(request, e, termIndex);
} catch (Throwable e) {
// For any Runtime exceptions, terminate OM.
Review Comment:
This comment needs to be updated.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -254,6 +257,17 @@ public void notifySnapshotInstalled(RaftProtos.InstallSnapshotResult result,
@Override
public TransactionContext startTransaction(
RaftClientRequest raftClientRequest) throws IOException {
+ if (readonlyMode.get()) {
+ TransactionContext ctxt = TransactionContext.newBuilder()
+ .setClientRequest(raftClientRequest)
+ .setStateMachine(this)
+ .setServerRole(RaftProtos.RaftPeerRole.LEADER)
+ .build();
+ ctxt.setException(new OMException("OM is running in readonly mode",
Review Comment:
This should specify that it is in this state due to an internal error. Otherwise it is not clear how the OM got into this state.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -4415,4 +4415,12 @@
maximum number of buckets across all volumes.
</description>
</property>
+ <property>
+ <name>ozone.om.onfailure.readonly.enable</name>
+ <value>false</value>
Review Comment:
Why not default to true?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -287,6 +301,11 @@ public TransactionContext preAppendTransaction(TransactionContext trx)
final OMRequest request = (OMRequest) trx.getStateMachineContext();
OzoneManagerProtocolProtos.Type cmdType = request.getCmdType();
+ if (readonlyMode.get()) {
Review Comment:
Why are we checking this in so many places? Prepare does a similar thing with just a pre-append check, since all write requests must go through there. I actually don't recall the difference between preAppend and startTransaction to know which is better for the use case in this change.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11116. OM state machine move to readonly mode on failure [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #6928:
URL: https://github.com/apache/ozone/pull/6928#discussion_r1688232384
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -287,6 +301,11 @@ public TransactionContext preAppendTransaction(TransactionContext trx)
final OMRequest request = (OMRequest) trx.getStateMachineContext();
OzoneManagerProtocolProtos.Type cmdType = request.getCmdType();
+ if (readonlyMode.get()) {
Review Comment:
I don't quite follow. Is this a performance improvement? I don't think we need to worry about that when every request in this state will fail. It would be simpler just to check in preAppend.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org