Fair enough. Opened up MAPREDUCE-990. Thanks. – Philip

* Do most of the getters that return arrays return null or an empty array if there's nothing there? This would be good to document in JavaDoc, since it's always confusing.

bq * Are the get*Timestamps functions public because they're used within the framework? It might be great for them to return int[] instead of String, and at least document what a "timestamp" indicates?

Please raise jira the suggested documentation changes

Amareshwari,

Sorry I slept through the first pass of this. A few questions about the new API:

• Do most of the getters that return arrays return null or an empty array if there's nothing there? This would be good to document in JavaDoc, since it's always confusing.

• Are the get*Timestamps functions public because they're used within the framework? It might be great for them to return int[] instead of String, and at least document what a "timestamp" indicates?

– Philip

Integrated in Hadoop-Mapreduce-trunk-Commit #19 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/19/)
. Changes DistributedCache to use the new API. Contributed by Amareshwari Sriramadasu.

I just committed this. Thanks, Amareshwari!

The changes look fine to me. +1 from my side.

-1 core tests. Is due to MAPREDUCE-943.
-1 javac. Is because of deprecation warnings.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12418580/patch-898-4.txt
against trunk revision 811134.

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

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

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

-1 javac. The applied patch generated 2236 javac compiler warnings (more than the trunk's current 2226 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 failed core unit tests.

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/37/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/37/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/37/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/37/console

This message is automatically generated.

There are some unsymmetric APIs. For e.g. getArchiveClassPaths() return Path objects while addCacheArchive() adds a URI. Same is the case with a missing setArchivesToClassPath(). Can be done in a follow up issue though.

Created MAPREDUCE-950

Patch with review comments incorporated.
I ran ant docs manually and verified document in mapred tutorial is fine.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12418467/patch-898-3.txt
against trunk revision 810505.

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

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

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

-1 javac. The applied patch generated 2236 javac compiler warnings (more than the trunk's current 2226 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/Mapreduce-Patch-h3.grid.sp2.yahoo.net/4/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/4/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/4/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/4/console

This message is automatically generated.

Some comments:
1) Move DistributedCache.checkURIs to TrackerDistributedCacheManager
2) Update the tutorial (forrest doc) with the new API related changes

Fixed javadoc warning

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12418362/patch-898-2.txt
against trunk revision 810330.

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

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

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

-1 javac. The applied patch generated 2236 javac compiler warnings (more than the trunk's current 2226 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 failed core unit tests.

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/3/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/3/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/3/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/3/console

This message is automatically generated.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12418353/patch-898-1.txt
against trunk revision 810330.

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

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

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

-1 javac. The applied patch generated 2230 javac compiler warnings (more than the trunk's current 2226 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 failed core unit tests.

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/2/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/2/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/2/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/2/console

This message is automatically generated.

Fixed the javadoc that Vinod pointed.

checkURIs() is the only API left in DistributedCache. If this can be moved somewhere too, perhaps to streaming which uses it, then the class DistributedCache itself can be deprecated.

Deprecated DistributedCache class itself. If users were using checkURIs() they will get a warning now.

Had a quick look at the patch. Few comments:

• This patch is not the cause, but there are many incorrect references to DistributedCacheManager in javadoc. Would be good if these are changed to TrackerDistributedCacheManager
• checkURIs() is the only API left in DistributedCache. If this can be moved somewhere too, perhaps to streaming which uses it, then the class DistributedCache itself can be deprecated.
• There are some unsymmetric APIs. For e.g. getArchiveClassPaths() return Path objects while addCacheArchive() adds a URI. Same is the case with a missing setArchivesToClassPath(). Can be done in a follow up issue though.

Moved setters specific to framework into TrackerDistributedCache. Made them package private

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12417958/patch-898.txt
against trunk revision 808730.

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

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

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

-1 javac. The applied patch generated 2230 javac compiler warnings (more than the trunk's current 2226 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 failed core unit tests.

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/25/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/25/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/25/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/25/console

This message is automatically generated.

-1 core tests. Due to MAPREDUCE-880 and MAPREDUCE-924
-1 javac. Is because of deprecated methods in DistributedCache.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12417958/patch-898.txt
against trunk revision 808730.

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

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

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

-1 javac. The applied patch generated 2230 javac compiler warnings (more than the trunk's current 2226 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 failed core unit tests.

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

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

This message is automatically generated.

Patch moves getters and setter from DistributedCache to JobContext and Job.