[
https://issues.apache.org/jira/browse/HDFS-12519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16196011#comment-16196011
]
Yiqun Lin edited comment on HDFS-12519 at 10/8/17 8:07 AM:
-----------------------------------------------------------
Thanks for updating the patch, [~nandakumar131].
bq. Used Time.monotonicNow() instead of Time.now()
Use Time.monotonicNow() for calculating elapsed time is right.
bq. This was done intentionally, so that the logging will be done as Lease
Okay with this.
bq. line65, line70 line154, line193: should add the check LOG.isDebug() before
invoking LOG.debug("....");
This comment seems not very clear. LOG.isDebug() is not always needed when
calling LOG.debug. If there are some parameters passed, the check LOG.isDebug()
is not needed. The usage:
{noformat}
LOG.debug("Releasing lease on {}", resource);
or
if(LOG.isDebugEnabled()) {
LOG.debug("Starting LeaseManager#LeaseMonitor Thread");
}
{noformat}
bq. We can't decide a proper interval which can suit for all the cases...
Thanks for sharing the thought. I am thinking if the method {{public
synchronized Lease<T> acquire(T resource, long timeout)}} is needed. Different
timeout value resource can just use separated lease manager to deal with.
Multiple timeout required resources make single lease manager inconveniently to
deal with.
If one lease manager deal with same timeout resource, LeaseMonitor#run can
simplify as fllowing:
{code}
public void run() {
while(monitor) {
LOG.debug("LeaseMonitor: checking for lease expiry");
for (T resource : activeLeases.keySet()) {
try {
Lease<T> lease = get(resource);
long remainingTime = lease.getRemainingTime();
if (remainingTime <= 0) {
//Lease has timed out
List<Callable<Void>> leaseCallbacks = lease.getCallbacks();
release(resource);
executorService.execute(
new LeaseCallbackExecutor(resource, leaseCallbacks));
}
} catch (LeaseNotFoundException | LeaseExpiredException ex) {
//Ignore the exception, someone might have released the lease
}
}
try {
Thread.sleep(defaultTimeout);
} catch (InterruptedException ignored) {
}
}
}
{code}
was (Author: linyiqun):
Thanks for updating the patch, [~nandakumar131].
bq. Used Time.monotonicNow() instead of Time.now()
Use Time.monotonicNow() for calculating elapsed time is right.
bq. This was done intentionally, so that the logging will be done as Lease
Okay with this.
bq. line65, line70 line154, line193: should add the check LOG.isDebug() before
invoking LOG.debug("....");
This comment seems not very clear. LOG.isDebug() is not always needed when
calling LOG.debug. If there are some parameters passed, the check LOG.isDebug()
is not needed. The usage:
{noformat}
LOG.debug("Releasing lease on {}", resource);
or
if(LOG.isDebugEnabled()) {
LOG.debug("Starting LeaseManager#LeaseMonitor Thread");
}
{noformat}
bq. We can't decide a proper interval which can suit for all the cases...
Thanks for sharing the thought. I am thinking if the method {{public
synchronized Lease<T> acquire(T resource, long timeout)}} is needed. Different
timeout value resource can just use separated lease manager to deal with.
Multiple timeout required resources make single lease manager inconveniently to
deal with.
> Ozone: Add a Lease Manager to SCM
> ---------------------------------
>
> Key: HDFS-12519
> URL: https://issues.apache.org/jira/browse/HDFS-12519
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Reporter: Anu Engineer
> Assignee: Nandakumar
> Labels: OzonePostMerge
> Attachments: HDFS-12519-HDFS-7240.000.patch,
> HDFS-12519-HDFS-7240.001.patch
>
>
> Many objects, including Containers and pipelines can time out during creating
> process. We need a way to track these timeouts. This lease Manager allows SCM
> to hold a lease on these objects and helps SCM timeout waiting for creating
> of these objects.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
