SequenceFile.Reader doesn't use the Serialization framework

• Key: HADOOP-3413
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Critical
• Assignee: Tom White
• Reporter: Arun C Murthy
• Component/s: io
• Affects Version/s: 0.17.0
• Fix Version/s: 0.18.0

Description

Currently SequenceFile.Reader only works with Writables, since it doesn't use the new Serialization framework. This is a glaring considering that SequenceFile.Writer uses the Serializer and handles arbitrary types via the SerializationFactory.

Comments

[Arun C Murthy]

Is this a 0.17.0 blocker?

[Arun C Murthy]

Uh, and SequenceFile{Input|Output}Format & SequenceFileRecord{Reader|Writer} still relies on Writables too...

[Tom White]

There is still clearly work to make the serialization framework fully supported in Hadoop, but I don't think the lack of support in SequenceFile.Reader is a blocker for 0.17.0.

The work done in HADOOP-1986 enabled serialization support in the MapReduce kernel, so you can use arbitrary types for keys and values. However, it is not yet possible to use arbitrary types for map inputs or reduce outputs out of the box, since the support from SequenceFile{Input|Output}Format and SequenceFileRecord{Reader|Writer} is still Writable-based as you point out. That said, it is possible to write your own InputFormat, OutputFormat, RecordReader, RecordWriter implementations to do this for you. For example, you can use SequenceFile.Writer#append(Object, Object) to write any objects to a sequence file (using a Serializer) and the SequenceFile.Reader#nextRaw methods to read bytes out to be manually deserialized using a Deserializer.

On a related note, unfortunately the RecordReader interface is incompatible with serialization frameworks that don't reuse objects - like Java Serialization. The problem is that

boolean next(K key, V value) throws IOException

has no way of passing keys and values that are deserialized from the stream back to the client of the RecordReader. This is not a problem for Writables and Thrift since the client passes in objects that are updated in-place. To fix this will require some surgery on the API.

[Devaraj Das]

I agree that this shouldn't be a blocker for 17 for the reasons Tom outlined.

[Tom White]

Here's a patch for extending SequenceFile.Reader, as well as making SequenceFile{Input|Output}Format and SequenceFileRecord{Reader|Writer} all use the Serialization framework. (I haven't tested it yet.)

[Arun C Murthy]

Tom, makes sense - thanks for weighing in!

The minor jarring note is that a simple app which writes a SequenceFile with Java Serialization won't be able to read it back...
On the bright side most Map-Reduce applications are ok since SequenceFile{Input|Output}Format doesn't support non-Writables yet.

[Tom White]

a simple app which writes a SequenceFile with Java Serialization won't be able to read it back...

But it would have to write its own SequenceFileOutputFormat as things stand, so while possible, it's not likely to be a common occurrence.

Thanks for picking this up Arun - I hope we can get this fixed in 0.18. There are also few other places that can remove their dependency on Writables without much effort - e.g. SequenceFileInputFilter, and MultipleOutputFormat (and subclasses).

[Enis Soztutar]

Please do not forget MapFile and related stuff.

[Tom White]

New patch to address SequenceFileInputFilter and MultipleOutputFormat. I'll open another issue for MapFile changes.

[Hadoop QA]

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12383235/hadoop-3413-v2.patch
against trunk revision 661918.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no tests are needed for this patch.

-1 javadoc. The javadoc tool appears to have generated 1 warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

+1 contrib tests. The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2538/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2538/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2538/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2538/console

This message is automatically generated.

[Tom White]

New patch to fix javadoc warnings. Existing tests ensure this change doesn't break anything.

[Owen O'Malley]

Doesn't this patch have the same problem that HADOOP-2997 had with the java serialization? It seems like the java serialization, which requires passing null the first time is pretty incompatible with a next method that looks like:

boolean next(K key, V value) throws IOException;

since clearly if you call it with next(null, null) you aren't going to be able to read your data. smile

[Tom White]

Owen,

I agree. This is a problem for the RecordReader interface, which will need work to change. However, I wasn't proposing we make such a radical change in this Jira, instead we are making it possible to use the serialization framework with SequenceFile.Reader, and removing unnecessary dependencies on Writable in some of the MapReduce layers (so you could at least use Thrift there). Note that the changes to SequenceFile.Reader are compatible with Java serialization, since instead of adding

public boolean next(Object key, Object val)

there's

public Object next(Object key)

and

public Object getCurrentValue(Object val)

[Owen O'Malley]

Ah, ok. So if I understand, this will let you read java.io sequence files, but not use it for map reduce input, right?

[Owen O'Malley]

I really think this should have unit tests, before it gets committed.

[Tom White]

Updated patch to test the new public API of SequenceFile.reader.

[Hadoop QA]

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12383547/hadoop-3413-v4.patch
against trunk revision 663889.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

+1 contrib tests. The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2606/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2606/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2606/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2606/console

This message is automatically generated.

[Owen O'Malley]

I just committed this, after fixing to reflect trunk changes. Thanks, Tom!

[Hudson]

Integrated in Hadoop-trunk #520 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/520/)