hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Loughran (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-7240) Object store in HDFS
Date Wed, 01 Nov 2017 21:25:04 GMT

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

Steve Loughran commented on HDFS-7240:
--------------------------------------

I'm starting with hadoop-common and hadoop-ozone; more to follow on thursday.

For now, biggest issue I have is that OzoneException needs to become an IOE, so simplifying
excpetion handling all round, preserving information, not losing stack traces, and generally
leading to happy support teams as well as developers. Changing the base class isn't itself
traumatic, but it will implicate the client code as there's almost no longer any need to catch
& wrap things.


Other: What's your scale limit? I see a single PUT for the upload, GET path > tmp in open()
. Is there a test for different sizes of file?

h2. hadoop-common

h3. Config


I've filed some comments on thecreated HADOOP-15007, "Stabilize and document Configuration
<tag> element", to cover making sure that there are the tests & docs for this to
go in.

* HDFSPropertyTag: s/DEPRICATED/r/DEPRECATED/
* OzonePropertyTag: s/there/their/ 
* OzoneConfig Property.toString() is going to be "key valuenull" if there is no tag defined.
Space?



h3. FileUtils

minor: imports all shuffled about compared to trunk & branch-2. revert.

h3. OzoneException

This is is own exception, not an IOE, and at least in OzoneFileSystem the process to build
an IOE from itinvariably loses the inner stack trace and all meaningful information about
the exception type. Equally, OzoneBucket catches all forms of IOException, converts to an
{{OzoneRestClientException}}. 

We don't need to do this. 

it will lose stack trace data, cause confusion, is already making the client code over complex
with catching IOEs, wrapping to OzoneException, catching OzoneException and converting to
an IOE, at which point all core information is lost. 

1. Make this subclass of IOE, consistent with the rest of our code, and then clients can throw
up untouched, except in the special case that they need to perform some form of exception.
1. Except for (any?) special cases, pass up IOEs raised in the http client as is.


Also.
* confused by the overridding of message/getmessage. Is for serialization? 
* Consider adding a setMessage(String format, string...args) and calling STring.format: it
would tie in with uses in the code.
* override setThrowable and setMessage() called to set the nested ex (hence full stack) and
handle the case where the exception returns null for getMessage().

{code}
OzoneException initCause(Throwable t) {
  super.initCause(t)
  setMessage(t.getMessage() != null ? t.getMessage() : t.toString())
}
{code}

h2. OzoneFileSystem

h3. general


* various places use LOG.info("text " + something). they should all move to LOG.info("text
{}", something)
* Once OzoneException -> IOE, you can cut the catch and translate here.
* qualify path before all uses. That's needed to stop them being relative, and to catch things
like someone calling ozfs.rename("o3://bucket/src", "s3a://bucket/dest"), delete("s3a://bucket/path"),
etc, as well as problems with validation happening before paths are made absolute.



* {{RenameIterator.iterate()}} it's going to log @ warn whenever it can't delete a temp file
because it doesn't exist, which may be a distraction in failures. Better: {{if(!tmpFile.delete()
&& tmpFile.exists())}}, as that will only warn if the temp file is actually there.


h3. OzoneFileSystem.rename(). 
Rename() is the operation to fear on an object store. I haven't looked at in full detail,.

* Qualify all the paths before doing directory validation. Otherwise you can defeat the "don't
rename into self checks"  rename("/path/src", "/path/../path/src/dest").
* Log @ debu all the paths taken before returning so you can debug if needed. 
* S3A rename ended up having a special RenameFailedException() which innerRename() raises,
with text and return code. Outer rename logs the text and returns the return code. This means
that all failing paths have an exception clearly thrown, and when we eventually make rename/3
public, it's lined up to throw exceptions back to the caller. Consider copying this code.

h3. OzoneFileSystem.delete

* qualify path before use
* dont' log at error if you can't delete a nonexistent path, it is used everywhere for silent
cleanup. Cut it

h3. OzoneFileSystem.ListStatusIterator

* make status field final

h3. OzoneFileSystem.mkdir

Liked your algorithm here; took me a moment to understand how rollback didn't need to track
all created directories. nice.
* do qualify path first.

h3. OzoneFileSystem.getFileStatus

{{getKeyInfo()}} catches all exceptions and maps to null, which is interpreted not found and
eventually surfaces as FNFE. This is misleading if the failure is for any other reason.

Once OzoneException -> IOException, {{getKeyInfo()}} should only catch & downgrade
the explicit not found (404?) responses.

h3. OzoneFileSystem.listKeys()

unless this needs to be tagged as VisibleForTesting, make private. 


h2. OzoneOutputStream

* Implement StreamCapabilities and declare that hsync/hflush are not supported.
* Unless there is no limit on the size of a PUT request/multipart uploads are supported, consider
having the 
stream's {{write(int)}} method fail when the limit is reached. That way, things will at least
fail fast.
* after close, set backupStream = null.
* {{flush()}} should be a no-op if called on a closed stream, so {{if (closed) return}}
* {{write()}} must fail if called on a closed stream,
* Again, OzoneException -> IOE translation which could/should be eliminated.


h2. OzoneInputStream

* You have chosen an interesting solution to the "efficient seek" problem here: D/L the entire
file and
then seek around. While this probably works for the first release, larger files will have
problems in both
disk space and size of 

* Again, OzoneException -> IOE translation which could/should be eliminated.

h2. Testing

* Implement something like {{AbstractSTestS3AHugeFiles}} for scale tests, again with the ability
to spec on the maven build how big the files to be created are. Developers should be able
to ask for a test run with an 8GB test write, read and seek, to see what happens.
* Add a subclass of {{org.apache.hadoop.fs.FileSystemContractBaseTest}}, ideally {{org.apache.hadoop.fs.FSMainOperationsBaseTest}}.
These test things which the newer contract tests haven't yet reimplimented.

h3. TestOzoneFileInterfaces

* Needs a {{Timeout}} rule for test timeouts.
* all your assertEquals strings are the wrong way round. sorry.


> Object store in HDFS
> --------------------
>
>                 Key: HDFS-7240
>                 URL: https://issues.apache.org/jira/browse/HDFS-7240
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>            Reporter: Jitendra Nath Pandey
>            Assignee: Jitendra Nath Pandey
>            Priority: Major
>         Attachments: HDFS-7240.001.patch, HDFS-7240.002.patch, HDFS-7240.003.patch, HDFS-7240.003.patch,
HDFS-7240.004.patch, Ozone-architecture-v1.pdf, Ozonedesignupdate.pdf, ozone_user_v0.pdf
>
>
> This jira proposes to add object store capabilities into HDFS. 
> As part of the federation work (HDFS-1052) we separated block storage as a generic storage
layer. Using the Block Pool abstraction, new kinds of namespaces can be built on top of the
storage layer i.e. datanodes.
> In this jira I will explore building an object store using the datanode storage, but
independent of namespace metadata.
> I will soon update with a detailed design document.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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


Mime
View raw message