You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2024/08/16 10:32:14 UTC
[PR] [CELEBORN-1568] Support worker retries in MiniCluster [celeborn]
cxzl25 opened a new pull request, #2692:
URL: https://github.com/apache/celeborn/pull/2692
### What changes were proposed in this pull request?
### Why are the changes needed?
https://github.com/apache/celeborn/actions/runs/10417785546/job/28852691241#step:4:5804
Now the worker retry logic, the first time of sleep is 2000, the second time is 4000000, and the third time is 8000000000 milliseconds. It is estimated that it will be difficult to complete the retry.
```scala
Thread.sleep(math.pow(2000, workerStartRetry).toInt)
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
GA
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1568] Support worker retries in MiniCluster [celeborn]
Posted by "FMX (via GitHub)" <gi...@apache.org>.
FMX closed pull request #2692: [CELEBORN-1568] Support worker retries in MiniCluster
URL: https://github.com/apache/celeborn/pull/2692
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1568] Support worker retries in MiniCluster [celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2692:
URL: https://github.com/apache/celeborn/pull/2692#discussion_r1719669102
##########
worker/src/test/scala/org/apache/celeborn/service/deploy/MiniClusterFeature.scala:
##########
@@ -202,16 +203,16 @@ trait MiniClusterFeature extends Logging {
logError(s"cannot start worker $i, reached to max retrying", ex)
throw ex
} else {
- Thread.sleep(math.pow(2000, workerStartRetry).toInt)
+ TimeUnit.SECONDS.sleep(Math.pow(2, workerStartRetry).toLong)
}
}
}
})
- workerThread.setName(s"worker ${i} starter thread")
+ workerThread.setName(s"worker $i starter thread")
workerThread
}
threads.foreach(_.start())
- Thread.sleep(5000)
+ Thread.sleep(15000)
Review Comment:
Now 2s+4s+8s=14s
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1568] Support worker retries in MiniCluster [celeborn]
Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #2692:
URL: https://github.com/apache/celeborn/pull/2692#discussion_r1720591951
##########
worker/src/test/scala/org/apache/celeborn/service/deploy/MiniClusterFeature.scala:
##########
@@ -202,16 +203,16 @@ trait MiniClusterFeature extends Logging {
logError(s"cannot start worker $i, reached to max retrying", ex)
throw ex
} else {
- Thread.sleep(math.pow(2000, workerStartRetry).toInt)
+ TimeUnit.SECONDS.sleep(Math.pow(2, workerStartRetry).toLong)
}
}
}
})
- workerThread.setName(s"worker ${i} starter thread")
+ workerThread.setName(s"worker $i starter thread")
workerThread
}
threads.foreach(_.start())
- Thread.sleep(5000)
+ Thread.sleep(15000)
Review Comment:
QQ: Why did we bump this from 5 -> 15 ?
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1568] Support worker retries in MiniCluster [celeborn]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2692:
URL: https://github.com/apache/celeborn/pull/2692#discussion_r1721005664
##########
worker/src/test/scala/org/apache/celeborn/service/deploy/MiniClusterFeature.scala:
##########
@@ -202,16 +203,16 @@ trait MiniClusterFeature extends Logging {
logError(s"cannot start worker $i, reached to max retrying", ex)
throw ex
} else {
- Thread.sleep(math.pow(2000, workerStartRetry).toInt)
+ TimeUnit.SECONDS.sleep(Math.pow(2, workerStartRetry).toLong)
}
}
}
})
- workerThread.setName(s"worker ${i} starter thread")
+ workerThread.setName(s"worker $i starter thread")
workerThread
}
threads.foreach(_.start())
- Thread.sleep(5000)
+ Thread.sleep(15000)
Review Comment:
Because `assert` is used here, it does not have a chance to retry before this PR.
https://github.com/apache/celeborn/blob/0ee3c3a4bdef3b97fca6be04f03ac36c2e12e1a6/worker/src/test/scala/org/apache/celeborn/service/deploy/MiniClusterFeature.scala#L228-L232
--
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@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org