You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/02/12 13:05:07 UTC
[PR] HDDS-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
swamirishi opened a new pull request, #7862:
URL: https://github.com/apache/ozone/pull/7862
## What changes were proposed in this pull request?
Currently when an apply transaction fails on ContainerStateMachine, the [isStateMachineHealthy](https://github.com/apache/ozone/blob/bf19af946aa08d2bea5064d79513a196b9bbf646/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java#L568) flag is set to false. However when the next set of transactions are applied this flag is not checked and all the transactions in the pipeline can get applied to the statemachine which could potentially bring down the statemachine in an inconsistent state. For instance if the write chunk fails and the next putBlock transaction succeeds then this would mean that the container is in an inconsistent state.
In this patch an unhealthyContainerSet is added to ensure no future transactions gets applied to the container if the container is present in the unhealthyContainerSet. A container would be added to containerSet if any errors are seen while writing StateMachine data/reading StateMachineData/applyTransactions to the container.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12236
## How was this patch tested?
Adding unit tests
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2675994701
> Having something in the raft log and not applying it & not having a txn in raft log is one and the same. Why have different states?
The difference is `relying on quorum` vs `need all the replicas` to close a QUASI_CLOSED Container. I agree that we are not in a good place right now to close a Container in QUASI_CLOSED state based on quorum, but that should not be the reason to add more code which will not allow us to use quorum based closing logic in future.
> I believe QUASI_CLOSED containers are containers which have missed a bunch of txns irrespective of it being in the raft log or not.
If that's the case, we don't even need Ratis. We can have a implementation without Ratis by having a static leader node and all the transactions can go through that leader and replicated to followers.
If we don't want to rely on Ratis quorum, we can actually simplify the flow by removing Ratis.
> Atleast the current implementation is so.
The current implementation has missed handling few failure cases, we should fix them.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1982594117
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
+ } catch (StorageContainerException e) {
+ return ContainerUtils.logAndReturnError(LOG, e, requestProto);
Review Comment:
done
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2675205130
> @swamirishi, thanks for the explanation.
>
> When there is a failure in `applyTransaction` and we add the Container to `unhealthyContainerSet`, no more transactions are applied to that Container.
>
> In this case, there could be committed transactions in the `Raft Log` which will never get applied to the `State Machine`. This means that there are transactions committed by this Datanode which will be missing in the Container.
>
> If that's the case, doesn't it make sense to mark the Container as `UNHEALTHY` rather than marking it as `QUASI_CLOSED`?
>
> From my understanding, `QUASI_CLOSED` state means that all the transactions committed (to `Raft Log`) by the Datanode is applied on the Container. `QUASI_CLOSED` replica might miss some transactions which the other replica has (if this is not the leader), but those transactions are not committed to the `Raft Log` in first place.
Having something in the raft log and not applying it & not having a txn in raft log is one and the same. Why have different states? If we move this to QUASI_CLOSED state we would be able to atleast close this container if all containers are in unhealthy state we won't be able to close it. If all replicas are in QUASI_CLOSED state the containers can be closed. I believe QUASI_CLOSED containers are containers which have missed a bunch of txns irrespective of it being in the raft log or not. Atleast the current implementation is so.
> If we go with this definition of `QUASI_CLOSED`, SCM might be able to Close a `QUASI_CLOSED` Container using quorum. That is, if we fix all the other issues on the Datanode.
>
> If we want to `QUASI_CLOSED` the Container, we should make sure that all the transaction committed to `Raft Log` are applied to the `State Machine`.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #7862:
URL: https://github.com/apache/ozone/pull/7862
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1965963102
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
Review Comment:
This function gets called on follower as well. We should not block other container writes. This will lead to a lot of container getting impacted because of one container failure.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2664698394
@swamirishi, thanks for identifying and working on the issue.
Instead of introducing new `unhealthyContainerSet`, why can't we mark the `Container Replica` as `UNHEALTHY` and send `ContainerAction` to SCM? By doing this we will be able to close the container, if the other two Replicas of the `Container` are in healthy state.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1965967818
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
+ } catch (StorageContainerException e) {
+ return ContainerUtils.logAndReturnError(LOG, e, requestProto);
Review Comment:
We should not fail. If we throw exception then the transactions to other containers won't get applied as commit index wouldn't increase. On exception throw we will fail all the transactions in future this would mean even close container will not make it. The only thing we are preventing here is failing writes prematurly preventing orphan block chunk writes. There should be no implication because of this.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1960007988
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
+ } catch (StorageContainerException e) {
+ return ContainerUtils.logAndReturnError(LOG, e, requestProto);
Review Comment:
We should follow same exception approach of catch and throw exception as done below for other failure. This need not follow applyTransaction way of handling, may have some impact.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
Review Comment:
IMO, If statemachine itself is unhealthy, we should reject any further writeStateMachineData(), so no need check for specific container. This is valid for applyTransaction() but not here.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1967209918
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
Review Comment:
as discussed, we should avoid additional request for the pipeline in writeData case. Application Transaction getting continue for the data already present is fine.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2679348481
> The difference is `relying on quorum` vs `need all the replicas` to close a QUASI_CLOSED Container. I agree that we are not in a good place right now to close a Container in QUASI_CLOSED state based on quorum, but that should not be the reason to add more code which will not allow us to use quorum based closing logic in future.
>
> >
This is an interim fix we should eventually get rid of the apply transaction block and perform all operations in the append transaction flow.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2706747272
Thanks for the reviews on the patch @nandakumar131 @sumitagrawl
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2675082073
@swamirishi, thanks for the explanation.
When there is a failure in `applyTransaction` and we add the Container to `unhealthyContainerSet`, no more transactions are applied to that Container.
In this case, there could be committed transactions in the `Raft Log` which will never get applied to the `State Machine`. This means that there are transactions committed by this Datanode which will be missing in the Container.
If that's the case, doesn't it make sense to mark the Container as `UNHEALTHY` rather than marking it as `QUASI_CLOSED`?
From my understanding, `QUASI_CLOSED` state means that all the transactions committed (to `Raft Log`) by the Datanode is applied on the Container. `QUASI_CLOSED` replica might miss some transactions which the other replica has (if this is not the leader), but those transactions are not committed to the `Raft Log` in first place.
If we go with this definition of `QUASI_CLOSED`, SCM might be able to Close a `QUASI_CLOSED` Container using quorum. That is, if we fix all the other issues on the Datanode.
If we want to `QUASI_CLOSED` the Container, we should make sure that all the transaction committed to `Raft Log` are applied to the `State Machine`.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7862:
URL: https://github.com/apache/ozone/pull/7862#issuecomment-2664779863
> @swamirishi, thanks for identifying and working on the issue. Instead of introducing new `unhealthyContainerSet`, why can't we mark the `Container Replica` as `UNHEALTHY` and send `ContainerAction` to SCM? By doing this we will be able to close the container, if the other two Replicas of the `Container` are in healthy state.
These containers are not unhealthy they should go into QUASI_CLOSED state technically. We should be just prevent any future transactions on these containers so that it doesn't get falsely applied and increases bcsid of the container.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1967208600
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
+ } catch (StorageContainerException e) {
+ return ContainerUtils.logAndReturnError(LOG, e, requestProto);
Review Comment:
As discuss over last meeting, need have similar exception handling for new and existing cases.
--
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-12236. ContainerStateMachine should not apply or write future transactions in the event of failure [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7862:
URL: https://github.com/apache/ozone/pull/7862#discussion_r1965971880
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java:
##########
@@ -554,6 +564,11 @@ private CompletableFuture<Message> writeStateMachineData(
CompletableFuture<ContainerCommandResponseProto> writeChunkFuture =
CompletableFuture.supplyAsync(() -> {
try {
+ try {
+ checkContainerHealthy(write.getBlockID().getContainerID());
Review Comment:
Since we are returning an error response, apply transaction will still go through there we wouldn't find the to be unhealthy container we might end applying a putBlock transaction without actually writing data.
--
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