Apache9 commented on a change in pull request #2890:
URL: https://github.com/apache/hbase/pull/2890#discussion_r560034538
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java
##########
@@ -141,28 +140,41 @@ public ChoreService(final String coreThreadPoolPrefix,
int corePoolSize, boolean
* @return true when the chore was successfully scheduled. false when the
scheduling failed
* (typically occurs when a chore is scheduled during shutdown of
service)
*/
- public synchronized boolean scheduleChore(ScheduledChore chore) {
+ public boolean scheduleChore(ScheduledChore chore) {
if (chore == null) {
return false;
}
-
- try {
- if (chore.getPeriod() <= 0) {
- LOG.info("Chore {} is disabled because its period is not positive.",
chore);
- return false;
- }
- LOG.info("Chore {} is enabled.", chore);
- chore.setChoreServicer(this);
- ScheduledFuture<?> future =
- scheduler.scheduleAtFixedRate(chore, chore.getInitialDelay(),
chore.getPeriod(),
- chore.getTimeUnit());
- scheduledChores.put(chore, future);
- return true;
- } catch (Exception exception) {
- if (LOG.isInfoEnabled()) {
- LOG.info("Could not successfully schedule chore: " + chore.getName());
+ // always lock chore first to prevent dead lock
+ synchronized (chore) {
Review comment:
The problem is the order, so it is OK that you synchronized
ScheduledChore first and then call the synchronized method in ChoreService.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
