You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "madhukara phatak (JIRA)" <ji...@apache.org> on 2012/07/03 11:01:13 UTC
[jira] [Created] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
madhukara phatak created HADOOP-8549:
----------------------------------------
Summary: Allow other implementation's of java.util.Map in MapWritable
Key: HADOOP-8549
URL: https://issues.apache.org/jira/browse/HADOOP-8549
Project: Hadoop Common
Issue Type: Improvement
Components: io
Affects Versions: 2.0.0-alpha
Reporter: madhukara phatak
Assignee: madhukara phatak
Priority: Minor
Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "madhukara phatak (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
madhukara phatak updated HADOOP-8549:
-------------------------------------
Status: Patch Available (was: Open)
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "madhukara phatak (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409229#comment-13409229 ]
madhukara phatak commented on HADOOP-8549:
------------------------------------------
Hi Harsha,
Thanks for looking at patch. Please find my reply for the concerns raised by you.
bq. 1. No javadocs or any other form of dev-docs seem to be present. How would others know how to use this feature? Lets document it.
Will add the appropriate javadoc for it once we finalize on implementation
bq. 2. I'm -1 on relying on protected member variables directly, as thats ugly to do. Do something similar to ArrayWritable instead, where an inheritor passes the class via a constructor. Or at least provide getter/setter of some form.
I tried it to make as ArrayWritable but it breaks the existing MapWritable code since implementation class information has to be written into stream. Even i tried setters/getters but it will not work since in M/R code Writables are created through default constructor. So protected member was only way i can think of solving the problem.
bq. The idea is good, but how does it work in terms of portability? If it is written by client who used X impl. of map, and then read by another client who used Y or the default impl., is it guaranteed to work? If yes, lets document that guarantee. And if not, does it throw a proper error message?
Since client has to use different class to get the new behavior (ex LinkedHashMapWritable) and Writables are not polymorphic, portability should not be an issue.
Regards,
Madhukara Phatak
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13405817#comment-13405817 ]
Hadoop QA commented on HADOOP-8549:
-----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12534668/HADOOP-8549.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:
org.apache.hadoop.ha.TestZKFailoverController
org.apache.hadoop.io.file.tfile.TestTFileByteArrays
org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1164//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1164//console
This message is automatically generated.
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "Harsh J (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414687#comment-13414687 ]
Harsh J commented on HADOOP-8549:
---------------------------------
Hi and thanks for the reply!
bq. I tried it to make as ArrayWritable but it breaks the existing MapWritable code since implementation class information has to be written into stream. Even i tried setters/getters but it will not work since in M/R code Writables are created through default constructor. So protected member was only way i can think of solving the problem.
{code}
/** {@inheritDoc} */
@Override
public void write(DataOutput out) throws IOException {
super.write(out);
// Write out the number of entries in the map
out.writeInt(instance.size());
// Then write out each key/value pair
for (Map.Entry<Writable, Writable> e: instance.entrySet()) {
out.writeByte(getId(e.getKey().getClass()));
e.getKey().write(out);
out.writeByte(getId(e.getValue().getClass()));
e.getValue().write(out);
}
}
{code}
In the above writer method, I do not see the instance's type being serialized. The key and value types are indeed stored, but not of the instance's.
Why can we hence not have a {{MapWritable(Class <? extends Map<Writable, Writable>> mapType)}} that then instantiates the class for the field member "instance", rather than a dev hacking like that?
bq. Since client has to use different class to get the new behavior (ex LinkedHashMapWritable) and Writables are not polymorphic, portability should not be an issue.
Lets still have a test case. Your current test case merely checks the ordering of a sequential implementation without serializing it. Lets instead serialize it, deserialize it again with your custom writable, check the ordering. Also deserialize it with a regular instance, and check the content. Let me know if this makes sense. This will help catch regressions in future and good tests are always good to have.
Thanks and let me know if am wrong somewhere or if you have any other questions! :)
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "madhukara phatak (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
madhukara phatak updated HADOOP-8549:
-------------------------------------
Attachment: HADOOP-8549.patch
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "Harsh J (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Harsh J updated HADOOP-8549:
----------------------------
Status: Open (was: Patch Available)
(Canceling until discussion's complete on this)
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "Harsh J (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409220#comment-13409220 ]
Harsh J commented on HADOOP-8549:
---------------------------------
Patch lacks a few items:
# No javadocs or any other form of dev-docs seem to be present. How would others know how to use this feature? Lets document it.
# I'm -1 on relying on protected member variables directly, as thats ugly to do. Do something similar to ArrayWritable instead, where an inheritor passes the class via a constructor. Or at least provide getter/setter of some form.
The idea is good, but how does it work in terms of portability? If it is written by client who used X impl. of map, and then read by another client who used Y or the default impl., is it guaranteed to work? If yes, lets document that guarantee. And if not, does it throw a proper error message?
Thanks!
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-8549) Allow other implementation's of
java.util.Map in MapWritable
Posted by "madhukara phatak (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-8549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414867#comment-13414867 ]
madhukara phatak commented on HADOOP-8549:
------------------------------------------
bq. Why can we hence not have a MapWritable(Class <? extends Map<Writable, Writable>> mapType) that then instantiates the class for the field member "instance", rather than a dev hacking like that?
AFAIK in M/R writable's are instantiated through default constructor, which means the above constructor will not be called. So if I want to have a custom Map instance implementation it has to be in read/write methods. Please correct me if I am wrong :)
> Allow other implementation's of java.util.Map in MapWritable
> ------------------------------------------------------------
>
> Key: HADOOP-8549
> URL: https://issues.apache.org/jira/browse/HADOOP-8549
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 2.0.0-alpha
> Reporter: madhukara phatak
> Assignee: madhukara phatak
> Priority: Minor
> Attachments: HADOOP-8549.patch
>
>
> Current implementation of MapWritable uses HashMap as Map implementation. But in some of the use cases we need other implementations of Map like LinkedHashMap,SortedMap.This jira changes visibility of 'instance' in MapWritable from private to protected which allows us to inject custom Map implementation through sub classing MapWritable .
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira