[
https://issues.apache.org/jira/browse/HADOOP-16279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16851821#comment-16851821
]
Gabor Bota commented on HADOOP-16279:
-------------------------------------
Thanks for the review [~fabbri], I fixed the issues you found.
A new PR is up. I managed to find and fix what the issue was: I was not
updating the last_updated field when the MS creates a tombstone. The issue with
that is the operation is done inside to MS, so I had to push ITtlTimeProvider
down to the MetadataStore interface, and I was not able to just add a new
static wrapper function to it in {{S3Guard}} as I originally wanted like
{{putWithTtl}} and {{getWithTtl}} - so the implementation is a little bit
asymmetric right now.
The main things to think about to proceed with this:
#
{{org.apache.hadoop.fs.s3a.commit.staging.integration.ITestDirectoryCommitMRJob}}
is failing ittermittently - I can't find the reason for that yet.
[~ste...@apache.org], could you like into it? This is the stacktrace:
{noformat}
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 100.874
s <<< FAILURE! - in
org.apache.hadoop.fs.s3a.commit.staging.integration.ITestDirectoryCommitMRJob
[ERROR]
testMRJob(org.apache.hadoop.fs.s3a.commit.staging.integration.ITestDirectoryCommitMRJob)
Time elapsed: 65.824 s <<< ERROR!
java.io.FileNotFoundException: Path
s3a://gabota-versioned-bucket-ireland/fork-0002/test/DELAY_LISTING_ME/testMRJob
is recorded as deleted by S3Guard
at
org.apache.hadoop.fs.s3a.S3AFileSystem.innerGetFileStatus(S3AFileSystem.java:2474)
at
org.apache.hadoop.fs.s3a.S3AFileSystem.getFileStatus(S3AFileSystem.java:2448)
at
org.apache.hadoop.fs.contract.ContractTestUtils.assertIsDirectory(ContractTestUtils.java:559)
at
org.apache.hadoop.fs.contract.AbstractFSContractTestBase.assertIsDirectory(AbstractFSContractTestBase.java:327)
at
org.apache.hadoop.fs.s3a.commit.AbstractITCommitMRJob.testMRJob(AbstractITCommitMRJob.java:133)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
at
org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
at
org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:748)
{noformat}
# Adding TTLTimeProvider to MetadataStore during initialization: right now I
pass {{ITtlTimeProvider}} to {{delete}}, {{deleteSubtree}} and {{move}}, but
not for {{get}} and {{put}} (I use S3Guard's static wrapper from
{{S3AFileSystem}} same as for auth directory ttl). This could change by adding
it to the metadatastore during initialization, and then it would handle the
expiry of metadata and authoritative directories internally as an
implementation detail.
# Default value for {{fs.s3a.metadatastore.metadata.ttl} - right now is 10
minutes in Constants. As we know this can take a while to stabilize, so I would
not say to set it less than 10 minutes (I had some test failures when setting
this to less then 3 minutes). What do you think about this, what should be the
default value?
# Prune tombstones CLI - I've added {{PruneMode}} enum to {{MetadataStore}}, so
in {{prune}} method you have to select what mode do you want the prune to run:
ALL_BY_MODTIME which is the default, old behaviour; TOMBSTONES_BY_LASTUPDATED
which is the new and you can use this to get rid of the old tombstones (e.g.
before a big folder rename/move) so get better performance. The S3Guard CLI is
public, so I haven't tuched that yet, but maybe in the future it would be a
good idea to add the availability to the CLI to select prune mode.
# More tests - if you find any execution path during the review which is not
tested yet but needs to be tested please tell me so I can add.
> S3Guard: Implement time-based (TTL) expiry for entries (and tombstones)
> -----------------------------------------------------------------------
>
> Key: HADOOP-16279
> URL: https://issues.apache.org/jira/browse/HADOOP-16279
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Reporter: Gabor Bota
> Assignee: Gabor Bota
> Priority: Major
> Attachments: Screenshot 2019-05-17 at 13.21.26.png
>
>
> In HADOOP-15621 we implemented TTL for Authoritative Directory Listings and
> added {{ExpirableMetadata}}. {{DDBPathMetadata}} extends {{PathMetadata}}
> extends {{ExpirableMetadata}}, so all metadata entries in ddb can expire, but
> the implementation is not done yet.
> To complete this feature the following should be done:
> * Add new tests for metadata entry and tombstone expiry to {{ITestS3GuardTtl}}
> * Implement metadata entry and tombstone expiry
> I would like to start a debate on whether we need to use separate expiry
> times for entries and tombstones. My +1 on not using separate settings - so
> only one config name and value.
> ----
> Notes:
> * In HADOOP-13649 the metadata TTL is implemented in LocalMetadataStore,
> using an existing feature in guava's cache implementation. Expiry is set with
> {{fs.s3a.s3guard.local.ttl}}.
> * LocalMetadataStore's TTL and this TTL is different. That TTL is using the
> guava cache's internal solution for the TTL of these entries. This is an
> S3AFileSystem level solution in S3Guard, a layer above all metadata store.
> * This is not the same, and not using the [DDB's TTL
> feature|https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/TTL.html].
> We need a different behavior than what ddb promises: [cleaning once a day
> with a background
> job|https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/howitworks-ttl.html]
> is not usable for this feature - although it can be used as a general
> cleanup solution separately and independently from S3Guard.
> * Use the same ttl for entries and authoritative directory listing
> * All entries can be expired. Then the returned metadata from the MS will be
> null.
> * Add two new methods pruneExpiredTtl() and pruneExpiredTtl(String keyPrefix)
> to MetadataStore interface. These methods will delete all expired metadata
> from the ms.
> * Use last_updated field in ms for both file metadata and authoritative
> directory expiry.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
