[ 
https://issues.apache.org/jira/browse/HDFS-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13853257#comment-13853257
 ] 

Colin Patrick McCabe commented on HDFS-5636:
--------------------------------------------

I think we should have a constant MAX_TTL_MS which is Long.MAX_VALUE / 4.  Then 
we should not accept either relative or absolute limits which are higher than 
that.  As you mentioned, these may cause overflows.  We should forbid them the 
same way we forbid negative times.  Then you could define 
RELATIVE_EXPIRY_NEVER_MS in terms MAX_TTL_MS.

We already encompass millions of years with MAX_VALUE / 4 milliseconds, so that 
limit should be fine...

{code}
  @Nullable
  Long maxRelativeExpiry;
{code}
should have "ms" at the end to indicate units.  Same goes for a lot of other 
variables here.

{code}
    // Allow if pool does not have a max expiry set,
    // fail if the requested expiry is greater than the max
    if (relExpiryTime > maxRelativeExpiryTime) {
      throw new InvalidRequestException("Expiration " + expiry.toString()
          + " exceeds the max relative expiration time of "
          + maxRelativeExpiryTime + " ms.");
    }
{code}
This comment seems a bit confusing.  Based on the first part, I looked for 
special handling for pools that don't have a max expiry, and didn't find it.  
Maybe it would be better to get rid of the first part of the comment, since 
"pools without a max expiry" are just pools with max expiry set to MAX, right?

Can we have a static {{Expiration.NEVER}} final constant which is an instance 
of {{Expiration}}?  Since Expiration is immutable, it would be easy, I think.  
Much nicer than making API users set the time to a huge value.

{code}
      if (maxTtlString != null) {
        long maxTtl;
        if (maxTtlString.equalsIgnoreCase("never")) {
          maxTtl = CachePoolInfo.RELATIVE_EXPIRY_NEVER;
        } else {
          try {
            maxTtl = DFSUtil.parseRelativeTime(maxTtlString);
          } catch (IOException e) {
            System.err.println(
                "Error while parsing maxTtl value: " + e.getMessage());
            return 1;
          }
        }
        info.setMaxRelativeExpiry(maxTtl);
{code}

This is duplicated a few times in CacheAdmin.  Seems like it should be a 
function.

> Enforce a max TTL per cache pool
> --------------------------------
>
>                 Key: HDFS-5636
>                 URL: https://issues.apache.org/jira/browse/HDFS-5636
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: caching, namenode
>    Affects Versions: 3.0.0
>            Reporter: Andrew Wang
>            Assignee: Andrew Wang
>         Attachments: hdfs-5636-1.patch
>
>
> It'd be nice for administrators to be able to specify a maximum TTL for 
> directives in a cache pool. This forces all directives to eventually age out.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)

Reply via email to