steveloughran commented on issue #802: HADOOP-16279. S3Guard: Implement 
time-based (TTL) expiry for entries …
URL: https://github.com/apache/hadoop/pull/802#issuecomment-497839845
 
 
   Code looks good, i dont' see any immediate issues. I do see that it'll 
conflict with my HADOOP-15183 code, because I too add a new argument to the put 
and move calls, and I've pushed addAncestor down. So whoever gets their code in 
first gets to dodge the merge pain.
   
   Added comments, mostly minor, but i've also been thinking of more tests.
   
   * switch to `Configuration.getTimeDuration()` in the time strings and then 
the docs, this avoids people wondering if a time is seconds, millis, etc. They 
get to choose.
   * And ideally, consider using ZonedDateTime in the TtlProvider to remove 
ambiguity there, though as its our own code, it's less critical. But, given we 
are assuming that everything is in UTC, including the other clients, I'd like 
this. {{insert anecdote about a machine have the wrong TZONE env var and 
cleaning up work dirs 8h ahead of when it should}}
   
   One thing we will need to make sure of is that when you add a file under a 
path where there is a tombstone marker, the file comes into existence and can 
be found in both get and list calls.
   
   - createNonRecursive must fail if the parent directory has been deleted, and 
succeed if the tombstone has expired
   - create(tombstone file) must succeed irrespective of overwrite flag
   - create("parent has tombstone") must always succeed (We dont check the 
parent), but after the file has been written, all entries up the tree must be 
valid. That is: the putAncestor code will correct everything.
   
   

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to