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 2025/11/22 13:35:31 UTC
[PR] HDDS-13981. DN quickly reregister to SCM [ozone]
sumitagrawl opened a new pull request, #9351:
URL: https://github.com/apache/ozone/pull/9351
## What changes were proposed in this pull request?
DN re-register quickly to SCM on HB response. It involves 2 step,
1. GET Version
2. Register
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13981
## How was this patch tested?
- Impacted test case
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564405108
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
We should never add `sleep` in tests as it makes them slow and flaky. Why is this needed here as the test class doesn't seem at all related to the 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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2581618360
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
assertThrows(IOException.class, c1::close); <-- close trigger **IOException** which internally call onFailure() which further results in **trigger for volume check**.
But this volume check is **ignored** as **diskCheckMinGap** is 2 second and this trigger is before 2 second time. So numOfFailedVolumes remain "0"
Timer waitFor() gets timeout causing failure of testcase since no further event to trigger volume check.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564561442
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
Its random failure currently, and with analysis, its requried to have 2 second diff between subsequent volume failure trigger.
So added sleep here to avoid that random success / failure.
Might be startup is quick now, Previous volume scanner trigger and next as required in test case have lesser gap and being observed. This is as SCM can come out safemode immediately with DN register quickly.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #9351:
URL: https://github.com/apache/ozone/pull/9351#issuecomment-3581782132
> > it feels like a hack instead of checking the node state (HB) the code is trying to trigger HB on for each GET VERSION which may have side effects when GET VERSION is called outside of register use case.
>
> * For other ICR, there is a different thread giving interrupt to HB thread to trigger immediate.
> * For this case, we can not do interrupt and State can not be exposed -- as defined State -- INIT, Running, Shutdown. Introducing new state is not recommended for next HB to trigger immediate.
>
> So updating HB time only is only solution without exposing internal action to HB mechanism, like an interrupt as being implemented for other threads.
setting up nextHB would not interrupt the current sleep cycle and would not have any effect to the thread that is sending HB. The right way of explicitly sending HB is to interrupt the HB thread and there is a method that does that.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564580001
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/endpoint/HeartbeatEndpointTask.java:
##########
@@ -425,6 +426,8 @@ private void processReregisterCommand() {
+ " Interrupt HEARTBEAT and transit to GETVERSION state.");
}
rpcEndpoint.setState(EndPointStates.GETVERSION);
+ // trigger immediate GETVERSION
+ context.getParent().setNextHB(Time.monotonicNow());
Review Comment:
As per flow, HB is send every 30 sec and these events are send with this timing. So it takes time to send GetVersion at 30th Sec after previous register asking register, and Register with next 30th second.
Volume Scan is not related to this HB interval, only during startup, this HB service starts after volume scan. But in both flow, it follow same interval, like 0th sec - GetVersion, 30th Sec - Register.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564561442
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
Its random failure currently, and with analysis, its requried to have 2 second diff between subsequent volume failure trigger.
So added sleep here to avoid that random success / failure.
Might be startup is quick now, Previous volume scanner trigger and next as required in test case have lesser gap and being observed.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9351:
URL: https://github.com/apache/ozone/pull/9351#issuecomment-3571879757
@yandrey321 & @ptlrs do you wanna review this 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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564561442
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
Its random failure currently, and with analysis, its requried to have 2 second diff between subsequent volume failure trigger.
So added sleep here to avoid that random success / 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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #9351:
URL: https://github.com/apache/ozone/pull/9351#issuecomment-3580854574
> it feels like a hack instead of checking the node state (HB) the code is trying to trigger HB on for each GET VERSION which may have side effects when GET VERSION is called outside of register use case.
- For other ICR, there is a different thread giving interrupt to HB thread to trigger immediate.
- For this case, we can not do interrupt and State can not be exposed -- as defined State -- INIT, Running, Shutdown. Introducing new state is not recommended for next HB to trigger immediate.
So updating HB time only is only solution without exposing internal action to HB mechanism, like an interrupt as being implemented for other threads.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564616534
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/endpoint/HeartbeatEndpointTask.java:
##########
@@ -425,6 +426,8 @@ private void processReregisterCommand() {
+ " Interrupt HEARTBEAT and transit to GETVERSION state.");
}
rpcEndpoint.setState(EndPointStates.GETVERSION);
+ // trigger immediate GETVERSION
+ context.getParent().setNextHB(Time.monotonicNow());
Review Comment:
Yes, Register is same to FCR only like first time register.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2580967527
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
Which line in the test was failing before the 2 second sleep?
Looking at the code, I suspect it was:
```
DatanodeTestUtils.waitForHandleFailedVolume(volSet, 1);
```
Rather than `assertThrows(IOException.class, c1::close);` as I think the latter line should pass immediately without any delay.
If it is hte waitForHandleFailedVolume part that is failing, why is it not waiting already?
```
public static void waitForHandleFailedVolume(
MutableVolumeSet volSet, int numOfFailedVolumes) throws Exception {
GenericTestUtils.waitFor(
() -> numOfFailedVolumes == volSet.getFailedVolumesList().size(),
100, 10000);
}
```
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #9351:
URL: https://github.com/apache/ozone/pull/9351#issuecomment-3572113047
it feels like a hack instead of checking the node state (HB) the code is trying to trigger HB on for each GET VERSION which may have side effects when GET VERSION is called outside of register use case.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2580155539
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
@ashishkumar50 @sodonnel
In Test MiniOzone cluster, diskCheckMinGap is configured as 2 second
`org.apache.hadoop.ozone.dn.volume.TestDatanodeHddsVolumeFailureDetection#newCluster --> dnConf.setDiskCheckMinGap(Duration.ofSeconds(2));`
So, if Bad volume is simulated failure, and previous run and next run gap is < 2 second, it ignores the validation.
In the testcase, its observed that volume check is triggered before marking the bad volume as default run, and after volume is simulated bad, next run with failure detection is less than 2 second. This was the cause of random failure as even volumeChecker is triggered, but its ignored due to this restriction.
This has started comming now as HB registration is quicker and DN is available for key create is faster.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2580155539
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
In Test MiniOzone cluster, diskCheckMinGap is configured as 2 second
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564441493
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/endpoint/HeartbeatEndpointTask.java:
##########
@@ -425,6 +426,8 @@ private void processReregisterCommand() {
+ " Interrupt HEARTBEAT and transit to GETVERSION state.");
}
rpcEndpoint.setState(EndPointStates.GETVERSION);
+ // trigger immediate GETVERSION
+ context.getParent().setNextHB(Time.monotonicNow());
Review Comment:
In the `VersionEndpointTask` it does not seem to set the same thing to immediately call register - is there a change needed there too, or is it not needed there for some reason?
On first startup, `VersionEndpointTask` might take a while as it looks like it scans all the volume as part of that, but after that is complete, we probably don't need to wait for another heartbeat interval. On a re-register, the volumes are likely already scanned so it should be fast, I think?
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2565377686
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -786,4 +786,8 @@ public Thread getCmdProcessThread() {
public VolumeChoosingPolicy getVolumeChoosingPolicy() {
return volumeChoosingPolicy;
}
+
+ public void setNextHB(long time) {
Review Comment:
setting nextHB would not interrupt current sleep interval, so it should be noop 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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl merged PR #9351:
URL: https://github.com/apache/ozone/pull/9351
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564665208
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
In the test code:
```
try {
DatanodeTestUtils.injectContainerMetaDirFailure(metadataDir);
// simulate bad volume by removing write permission on root dir
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
// close container to trigger checkVolumeAsync
assertThrows(IOException.class, c1::close);
```
SimulateBadVolume is DN side code and c1::close is also DN code. There should be no need to sleep inbetween both of these calls. I don't see how the change in this PR has triggered this problem, and in general we need to come up with a better way of dealing with delays that putting a sleep in. There will be a time when the 2 seconds is not enough and it causes flaky failures. I think this failiure, if caused by this PR needs investigate a bit more to see what is going on.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2579827535
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
@sumitagrawl Could you please elaborate more on why 2sec sleep is required here? It can still lead to flaky in future if something else is changed. Instead we should we wait on certain condition to avoid flakiness also reduce the test time.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2580155539
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
@ashishkumar50 @sodonnel
In Test MiniOzone cluster, diskCheckMinGap is configured as 2 second
`org.apache.hadoop.ozone.dn.volume.TestDatanodeHddsVolumeFailureDetection#newCluster --> dnConf.setDiskCheckMinGap(Duration.ofSeconds(2));`
So, if Bad volume is simulated failure, and previous run and next run gap is < 2 second, it ignores the validation. This volume checker is dynamically triggered based on failure as in testcase and expected to report failure.
In the testcase, its observed that volume check is triggered before marking the bad volume as default run, and after volume is simulated bad, next run with failure detection is less than 2 second. This was the cause of random failure as even volumeChecker is triggered, but its ignored due to this restriction.
This has started comming now as HB registration is quicker and DN is available for key create is faster.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2564606746
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/endpoint/HeartbeatEndpointTask.java:
##########
@@ -425,6 +426,8 @@ private void processReregisterCommand() {
+ " Interrupt HEARTBEAT and transit to GETVERSION state.");
}
rpcEndpoint.setState(EndPointStates.GETVERSION);
+ // trigger immediate GETVERSION
+ context.getParent().setNextHB(Time.monotonicNow());
Review Comment:
So do we need another similar change after the register, or does the register container the FCR effectively bringing the DN into normal service?
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2565390417
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
its better to use wait with max timeout versus a single sleep.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2568689445
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -786,4 +786,8 @@ public Thread getCmdProcessThread() {
public VolumeChoosingPolicy getVolumeChoosingPolicy() {
return volumeChoosingPolicy;
}
+
+ public void setNextHB(long time) {
Review Comment:
Thread sleep is based on ((time+30) - current time). (time+30) is set before processing, so setting (time+30) back to "time" will have negligible sleep. So this will work 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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #9351:
URL: https://github.com/apache/ozone/pull/9351#issuecomment-3585861278
> setting up nextHB would not interrupt the current sleep cycle and would not have any effect to the thread that is sending HB. The right way of explicitly sending HB is to interrupt the HB thread and there is a method that does that.
Can we interrupt from the same thread which is going to sleep later on? Not possible and can be done by other thread. Since there is no other thread, so controlling sleep is the better way.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2569569373
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
I don't understand why any wait is needed here. The `injectContainerMetaDirFailure` and `simulateBadVolume` immediately set the permissions on the folder to be non-writable, so the closing of the container should fail immediately. There isn't anything asynchronous going on here that I can see.
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2584153897
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/volume/TestDatanodeHddsVolumeFailureDetection.java:
##########
@@ -156,7 +156,8 @@ void corruptContainerFile(boolean schemaV3) throws Exception {
// refer to HddsVolume.check()
DatanodeTestUtils.simulateBadVolume(vol0);
- // close container to trigger checkVolumeAsync
+ // close container to trigger checkVolumeAsync after 2 seconds as minGap to check
+ Thread.sleep(2000);
Review Comment:
@sodonnel updated the PR with diskCheckMinGap "0" and revert sleep
--
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-13981. DN quickly reregister to SCM [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #9351:
URL: https://github.com/apache/ozone/pull/9351#discussion_r2584156429
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -786,4 +786,8 @@ public Thread getCmdProcessThread() {
public VolumeChoosingPolicy getVolumeChoosingPolicy() {
return volumeChoosingPolicy;
}
+
+ public void setNextHB(long time) {
Review Comment:
@yandrey321 updated the PR with setNextHB() comment for the usecase
--
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