[
https://issues.apache.org/jira/browse/HADOOP-13761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16367653#comment-16367653
]
Steve Loughran commented on HADOOP-13761:
-----------------------------------------
Spent a lot of time staring at that read code, good to follow those failure
modes. I like you did with the fault injection.
What this adds is not just the s3guard FNFE handling, but the whole resilience
to read failures. Which is nice.
Some thoughts
* Mark up the seek/read calls with @RetryPolicy, so it's easier to see what
happens transitively
* only have the s3guardInvoker field set to S3GuardExistsRetryPolicy if S3Guard
is enabled, otherwise just set {{s3guardInvoker=invoker}}.
* and maybe just call it {{readInvoker}}
* then you can go {{context.readInvoker}} on those calls without much logic in
the input stream.
* the invoked operations themselves can be stuck inline
BTW, one thing I don't remember us having any test for is the code sequence
{code}
Path p = path();
writeTextFile(fs, p, "hello")
InputStream s = fs.open(path)
fs.delete(p)
intercept(FileNotFoundException.class, () -> s.read())
{code}
This would let us check that the deletion of a file does still get picked up in
a bounded-time when s3guard is turned on.
+ same for readfully().
A really clever variant of that test would be a test which did the delete after
the first read and before a seek we knew would trigger a new GET (such as a
backwards one). Again, we'd expect a failure with/without s3guard.
Finally, you know, we don't actually implement skip() ourselves in S3A, just do
a read + discard all the way to the end. Filed HADOOP-15245 to do that later.
Luckily, none of the columnar formats appear to use skip(), or, if anything
uses skip, it's doing it over short distances. Whatever gets done there should
think about resilience testing too.
> S3Guard: implement retries for DDB failures and throttling; translate
> exceptions
> --------------------------------------------------------------------------------
>
> Key: HADOOP-13761
> URL: https://issues.apache.org/jira/browse/HADOOP-13761
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.0.0-beta1
> Reporter: Aaron Fabbri
> Assignee: Aaron Fabbri
> Priority: Blocker
> Attachments: HADOOP-13761-004-to-005.patch,
> HADOOP-13761-005-to-006-approx.diff.txt, HADOOP-13761-005.patch,
> HADOOP-13761-006.patch, HADOOP-13761.001.patch, HADOOP-13761.002.patch,
> HADOOP-13761.003.patch, HADOOP-13761.004.patch
>
>
> Following the S3AFileSystem integration patch in HADOOP-13651, we need to add
> retry logic.
> In HADOOP-13651, I added TODO comments in most of the places retry loops are
> needed, including:
> - open(path). If MetadataStore reflects recent create/move of file path, but
> we fail to read it from S3, retry.
> - delete(path). If deleteObject() on S3 fails, but MetadataStore shows the
> file exists, retry.
> - rename(src,dest). If source path is not visible in S3 yet, retry.
> - listFiles(). Skip for now. Not currently implemented in S3Guard. I will
> create a separate JIRA for this as it will likely require interface changes
> (i.e. prefix or subtree scan).
> We may miss some cases initially and we should do failure injection testing
> to make sure we're covered. Failure injection tests can be a separate JIRA
> to make this easier to review.
> We also need basic configuration parameters around retry policy. There
> should be a way to specify maximum retry duration, as some applications would
> prefer to receive an error eventually, than waiting indefinitely. We should
> also be keeping statistics when inconsistency is detected and we enter a
> retry loop.
--
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
