You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "He Yongqiang (JIRA)" <ji...@apache.org> on 2009/03/23 07:56:50 UTC
[jira] Created: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
---------------------------------------------------------------------------------------------------------
Key: HADOOP-5553
URL: https://issues.apache.org/jira/browse/HADOOP-5553
Project: Hadoop Core
Issue Type: Improvement
Reporter: He Yongqiang
SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "Hong Tang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689155#action_12689155 ]
Hong Tang commented on HADOOP-5553:
-----------------------------------
bq. Since SequenceFile provides a public methos nextRawValue(ValueBytes val), which accpets a ValueBytes param. And also ValueBytes is a public interface. So it seems it allows users to define their own ValueBytes implementations. But in nextRawValue(ValueBytes val), it casts the passed param into either CompressedBytes and UnCompressedBytes. I do not think it makes any sence.
I think the current interface is a good example of how to hide information through the use of interface. The intention of this interface is to allow people to pass value bytes to an outputstream when people do not need to examine the deserialized object. In the meantime, it hides the implementation detail that it is implemented by eagerly reading bytes into a byte array.
bq. Actually i am trying to skip some bytes of the value part of one record, and not lazy load. Is there a possibility to allow SequenceFile to extend to support this?
That is a valid concern, but have you profiled your code that skipping bytes is indeed the right place you want to optimize for? Have you tried the alternative of obtaining the serialized value bytes through a ByteArrayOutputStream, and then do skipping from byte[]?
Finally, a different way of achieving this goal (accessing raw bytes in read-only fashion without exposing implementation details) is to return an InputStream for the value bytes. This is the approach taken by Hadoop-3315.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
He Yongqiang updated HADOOP-5553:
---------------------------------
Attachment: Hadoop-5553.patch
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "Chris Douglas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Chris Douglas resolved HADOOP-5553.
-----------------------------------
Resolution: Won't Fix
Disallowing user extension here is exactly the right choice. An interface to a family of binary readers for a file format adds another layer of support, and I can imagine no other use than the one you propose: avoiding the full read into memory.
Instead of making these inner classes public, perhaps this functionality could be added to SequenceFile. Then the community benefits, rather than being burdened with its maintenance.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688605#action_12688605 ]
He Yongqiang commented on HADOOP-5553:
--------------------------------------
Actually i am trying to skip some bytes of the value part of one record, and not lazy load.
Is there a possibility to allow SequenceFile to extend to support this?
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
He Yongqiang updated HADOOP-5553:
---------------------------------
Attachment: Hadoop-5553-3.patch
Also change the set() method of both CompressedBytes and UnCompressedBytes to public. This will allow subclasses to override its behaviour.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "Chris Douglas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688896#action_12688896 ]
Chris Douglas commented on HADOOP-5553:
---------------------------------------
The binary reader for a file format doesn't seem like the right place to inject record projection or record skipping, particularly through user code. if the reader were modified to read lazily, then whatever is consuming the value bytes can discard whatever it pleases from the stream.
Are you sure that SequenceFile fulfills your requirements? If you need such specific control, perhaps a less general format would be more fitting.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
He Yongqiang updated HADOOP-5553:
---------------------------------
Attachment: (was: Hadoop-5553-3.patch)
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
He Yongqiang updated HADOOP-5553:
---------------------------------
Attachment: Hadoop-5553-3.patch
Hadoop-5553-3.patch is now a version to the truck. Previous Hadoop-5553-3 was deleted.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
He Yongqiang updated HADOOP-5553:
---------------------------------
Attachment: Hadoop-5553-2.patch
Hadoop-5553-2.patch also change the constructs' modifier from private to public
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688532#action_12688532 ]
He Yongqiang commented on HADOOP-5553:
--------------------------------------
Hi, Chris. I don't agree with you.
Since SequenceFile provides a public methos nextRawValue(ValueBytes val), which accpets a ValueBytes param. And also ValueBytes is a public interface. So it seems it allows users to define their own ValueBytes implementations. But in nextRawValue(ValueBytes val), it casts the passed param into either CompressedBytes and UnCompressedBytes. I do not think it makes any sence.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "He Yongqiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689159#action_12689159 ]
He Yongqiang commented on HADOOP-5553:
--------------------------------------
Thanks, Hong. Skipping unneeded data is indeed not so expensive. This is an initial demand for my try on implementing hive-352. What Chris said is right, this is a very specific need and should not place such demands on a common binary format. Thanks, Hong. I will take your suggestions into the implementation.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-5553) Change modifier of
SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from
private to public
Posted by "Chris Douglas (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-5553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688568#action_12688568 ]
Chris Douglas commented on HADOOP-5553:
---------------------------------------
I'll try to explain my position.
The SequenceFile.Reader instance creates the instance passed to nextRawValue. It is contrary to its design to pass an arbitrary ValueBytes object in, as it cannot be guaranteed that it observes the semantics the Reader expects (which is why the instance is cast to a subclass in nextRaw\*). If exchanging ValueBytes instances between Readers is not guaranteed, then supporting more general user code certainly should not be.
Consider how much wider the interface becomes when user code is permitted. Right now, SequenceFile.Reader is threadsafe, because the full record is consumed in nextRaw. If a lazy reader were to block that stream until the full value were consumed, it would introduce the possibility of deadlock (if it didn't, its results would be undefined). Lazy vbytes A might block, while lazy vbytes B may page to disk if there's contention. Threads might use a mix of ValueBytes instances into the same Reader, some written by the user, others from library code. In my mind, lazily loading ValueBytes is a worthy feature for SequenceFile.Reader, not one of a set of possible extensions to a binary interface. Again, I can't think of a second one.
As an alternative, consider implementing {{createValueBytes(boolean lazy)}}, returning a ValueBytes instance that lazily reads the value. This permits SequenceFile to initialize any locks/state necessary to support this, keeps the contract contained in SequenceFile, and adds only one parameter to an advanced interface.
> Change modifier of SequenceFile.CompressedBytes and SequenceFile.UncompressedBytes from private to public
> ---------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5553
> URL: https://issues.apache.org/jira/browse/HADOOP-5553
> Project: Hadoop Core
> Issue Type: Improvement
> Reporter: He Yongqiang
> Attachments: Hadoop-5553-2.patch, Hadoop-5553-3.patch, Hadoop-5553.patch
>
>
> SequenceFile.rawValue() provides the only interface to navigate the underlying bytes. And with some little work on implementing a customized ValueBytes can avoid reading all bytes into memory. Unfortunately, the current nextRawValue will cast the passing ValueBytes to either private class CompressedBytes or private class UnCompressedBytes, this will disallow user further extension.
> I can not see any reason that CompressedBytes and UnCompressedBytes should be set to private. And since the ValueBytes is public and nextValue() casts it to either CompressedBytes or UnCompressedBytes, i think it would be better if they are public.
> I am stuck now by this issue, really appracited if this got resolved as soon as possible.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.