hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Loughran (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HADOOP-13651) S3Guard: S3AFileSystem Integration with MetadataStore
Date Mon, 31 Oct 2016 18:45:59 GMT

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

Steve Loughran edited comment on HADOOP-13651 at 10/31/16 6:45 PM:
-------------------------------------------------------------------

comments on patch 003

{{LocalMetadataStore.listChildren(). get(), put}} are going to be *very* expensive as the
iterative loop is called even when the logging doesn't take place. Wrap with a LOG.isDebugEnabled(),
or have {{DirListingMetadata}} implement a toString() method with all the log info, and just
reference that.


There's a fairly complex interdependency between S3AFS and the metadatastore (now that LocalMetadataStore
is checking for its FS being S3A). The is empty dir logic here is getting ugly already. I
think it may be best to revisit that entire empty-dir logic and see how it can be moved into
this world, rather than just making things more complex. Currently, there's a couple of main
ways the {{isEmptyDirectory}} bit is used 

* when deciding whether to delete an entry during parent delete walks {{deleteUnnecessaryFakeDirectories}}.
This code has already been replaced in trunk.
* when validating some operations which only apply to an empty directory. It's essentially
being a shortcut to for the predicate "has-no-children", which again, is expensive in s3-land.
If, instead of asking of the s3a status, it was just something which could be queried off
the metadata store, then it gets to implement the logic behind {{S3Guard.isEmptyDirectory(metadatastore,
s3afilestatus)}}
  
The base metadatastore (which would have to be renamed something like DefaultMetadataStore)
would implement its check by passthrough from the file status:
{code}
boolean isEmptyDirectory(S3AFileStatus stat) { return stat.isEmptyDirectory;}
{code}

Other implementations can actually do a listing. It should be possible to require that there
should be no accesses of that flag in the status except through an MD store class. ({{S3AFileStatus}})
is tagged as private/evolving, no external code should be using that field.

Finally, now that this starts hooking up to S3, it's going to need to have a security story
consistent with S3A. Which is currently: you get R/O or R/W filesystems, as well as filesystems
an unauthed user may not read. We expect all FS operations to fail on an unauthed user; if
they have read only rights then mkdirs/delete, rename and file writing must all fail, leaving
the FS in the same state it was before. Which implies that (a) there will have to be isolation
between users and (b) things which update the MD store after, say , "delete", will have to
take place after the s3 call succeeds, doing nothing on a failure.

That should be testable: try to delete the landsat CSV, verify that it is still there on the
next list/stat/open. Do bear in mind that other test infras may not have that file, or supply
one in an R/W bucket ( a new complication, given there aren't yet any tests for attempting
a write in an R/O bucket).


was (Author: stevel@apache.org):
comments on patch 003

{{LocalMetadataStore.listChildren(). get(), put}} are going to be *very* expensive as the
iterative loop is called even when the logging doesn't take place. Wrap with a LOG.isDebugEnabled(),
or have {{DirListingMetadata}} implement a toString() method with all the log info, and just
reference that.


There's a fairly complex interdependency between S3AFS and the metadatastore (now that LocalMetadataStore
is checking for its FS being S3A). The is empty dir logic here is getting ugly already. I
think it may be best to revisit that entire empty-dir logic and see how it can be moved into
this world, rather than just making things more complex. Currently, there's a couple of main
ways the {{isEmptyDirectory}} bit is used 

* when deciding whether to delete an entry during parent delete walks {{deleteUnnecessaryFakeDirectories}}.
This code has already been replaced in trunk.
* when validating some operations which only apply to an empty directory. It's essentially
being a shortcut to for the predicate "has-no-children", which again, is expensive in s3-land.
If, instead of asking of the s3a status, it was just something which could be queried off
the metadata store, then it gets to implement the logic.

i.e  {{S3Guard.isEmptyDirectory(metadatastore, s3afilestatus).
  
The base metadatastore (which would have to be renamed something like DefaultMetadataStore)
would implement its check by passthrough from the file status: {{boolean isEmptyDirectory(S3AFileStatus
stat) { return stat.isEmptyDirectory;} }}. Other implementations can actually do a listing.
It should be possible to require that there should be no accesses of that flag in the status
except through an MD store class. ({{S3AFileStatus}}) is tagged as private/evolving, no external
code should be using that field.

Finally, now that this starts hooking up to S3, it's going to need to have a security story
consistent with S3A. Which is currently: you get R/O or R/W filesystems, as well as filesystems
an unauthed user may not read. We expect all FS operations to fail on an unauthed user; if
they have read only rights then mkdirs/delete, rename and file writing must all fail, leaving
the FS in the same state it was before. Which implies that (a) there will have to be isolation
between users and (b) things which update the MD store after, say , "delete", will have to
take place after the s3 call succeeds, doing nothing on a failure.

That should be testable: try to delete the landsat CSV, verify that it is still there on the
next list/stat/open. Do bear in mind that other test infras may not have that file, or supply
one in an R/W bucket ( a new complication, given there aren't yet any tests for attempting
a write in an R/O bucket).

> S3Guard: S3AFileSystem Integration with MetadataStore
> -----------------------------------------------------
>
>                 Key: HADOOP-13651
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13651
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>            Reporter: Aaron Fabbri
>            Assignee: Aaron Fabbri
>         Attachments: HADOOP-13651-HADOOP-13345.001.patch, HADOOP-13651-HADOOP-13345.002.patch,
HADOOP-13651-HADOOP-13345.003.patch
>
>
> Modify S3AFileSystem et al. to optionally use a MetadataStore for metadata consistency
and caching.
> Implementation should have minimal overhead when no MetadataStore is configured.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message