|
The attached patch (which includes the patch for
Note that this patch probably will be conflicting in Hudson until I test the patch OK.And I suggest to add the group attribute in the extra data
the metric in Ganglia3.1.* has the attribute,so i think to put the prefix of the metric name to the group name. for example: metric :dfs.namenode.FilesCreated --> group: dfs.namenode Hey zhangwei,
The group attribute stuff is a good improvement. Have you tested it on your cluster? I'll perhaps get to do this today or tomorrow; we're going to be upgrading to Hadoop 0.19.0 soon, which will allow me to slip a few new patches in. Brian Hi, Brian
I tested it on my cluster,which has ganglia 3.1.1 installed.I add a little code simply based on your patch: + String GroupName = name.substring(0,name.lastIndexOf(".")) And it seems OK. I can confirm that the alterations to my patch do indeed appear to work.
Submitting patch to Hudson.
+ InetAddress localMachine = InetAddress.getLocalHost(); + hostName = localMachine.getHostName(); For 0.20 and onwards, this patch should use the code in org.apache.hadoop.net.DNS, rather than try and do this itself. The DNS class has caching and fallback for machines that don't know who they are. Hey Steve,
1) This does not break Ganglia 3.0. It gives a subclass of the Ganglia 3.0 implementation that is compatible with 3.1. 2) I'll have to work on this - we're currently using the patch for 0.19, so I haven't looked at the DNS class yet. Can you give an example (or fix the patch yourself) I believe this works - Steve, can you look over my usage of the DNS class?
Ok - I think the patch is ready - why is there no "Submit Patch" from JIRA? Did I do something wrong?
Hm - found a bug in the v5 patch. This next one currently uses the DNS class correctly as Steve suggested.
This next one includes a test case for Ganglia – it's quite simple. All it does is configure the MiniDFSCluster to report to Ganglia31, and then starts up a separate thread which listens for metrics. It verifies that the hostname is properly set inside the packet sent to Ganglia, but does not verify that all the formatting is correct. This appears to be the first test for Ganglia? Probably should have written it awhile ago. Looks good to me, +1.
One thing to consider Ok, submitting version 6 to Hudson. Includes a test case and correct DNS usage. Go Hudson, go!
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12404072/HADOOP-4675-v6.patch  against trunk revision 763728. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/174/console This message is automatically generated. v7 of the patch - I think I've fixed the 'patch' issue in Hudson - just a bum development environment on my laptop.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12407207/HADOOP-4675-v7.patch against trunk revision 772688. +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 does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 3 new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. -1 release audit. The applied patch generated 476 release audit warnings (more than the trunk's current 475 warnings). -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/296/testReport/ This message is automatically generated. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12407207/HADOOP-4675-v7.patch against trunk revision 772844. +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 does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 3 new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +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 failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/299/testReport/ This message is automatically generated. Could you have a look at these findbugs warnings and see if it is possible to fix those?
add support for Ganglia 3.1 metrics
These minor changes in this[1] patch should fix the findbugs warnings in previous versions. Tested against rev 805676 (current trunk)
[1] https://issues.apache.org/jira/secure/attachment/12416987/HADOOP-4675-v8.patch -------- +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12416987/HADOOP-4675-v8.patch against trunk revision 804918. +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 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-vesta.apache.org/613/testReport/ This message is automatically generated. Any word on whether this is in the trunk now? Thanks!
Integrated in Hadoop-Common-trunk-Commit #9 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/9/
)Current Ganglia metrics implementation is incompatible with Ganglia 3.1 Integrated in Hadoop-Common-trunk-Commit #10 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/10/
)Current Ganglia metrics implementation is incompatible with Ganglia 3.1 I applied the patch but then noticed that the unit test isn't being run; its got the wrong package on it and so applies outside of src/test/core. It then fails to compile because it uses minidfscluster it in the 'correct' location – under src/test/core. I reversed the patch until this is addressed.
Integrated in Hadoop-Common-trunk-Commit #11 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/11/
)Current Ganglia metrics implementation is incompatible with Ganglia 3.1 – reversing this patch Trivial change to drop TestGangliaContext31 into src/test/core instead of src/test per stack's request.
Thanks Matt, you beat me to it.
Thanks lads.
@Matt: Does it compile for you? I get this when I apply to current common TRUNK: [javac] Compiling 126 source files to /Users/stack/checkouts/common/trunk/build/test/core/classes
[javac] /Users/stack/checkouts/common/trunk/src/test/core/org/apache/hadoop/metrics/TestGangliaContext31.java:31: package org.apache.hadoop.hdfs does not exist
[javac] import org.apache.hadoop.hdfs.MiniDFSCluster;
[javac] ^
Here's my version: URL: https://svn.apache.org/repos/asf/hadoop/common/trunk Repository Root: https://svn.apache.org/repos/asf Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68 Revision: 811214 ... Common doesn't bundle hdfs. I'm doing something wrong or the test no longer suits after the refactoring of hadoop into different modules. If no minidfscluster available in common, then the test doesn't fit the new constrained environment (though the test is creative with its setup of a listener to catch and verify the ganglia emissions). I'd be grand w/ removing the test. What you all think? @stack: the patch compiles if you run, e.g...
CLASSPATH="../hadoop-hdfs/build/hadoop-hdfs-0.21.0-dev.jar:../hadoop-hdfs/build/hadoop-hdfs-test-0.21.0-dev.jar" ant bin-package You're correct that MiniDFSCluster is not a part of hadoop-common but rather hadoop-hdfs. In the future, I will run something like the following to test patches since I think it closely mimics Hudson, e.g. ant test-patch -Dpatch.file=HADOOP-4675-v9.patch -Dfindbugs.home=... -Djava5.home=... -Dforrest.home=... where none of the non-hadoop-common jars are in the classpath. I read through your test and agree: it is creative. It's a shame that MiniDFSCluster isn't available in common. Maybe the test should be commented out until Mini clusters are available for common tests? It's not ideal but having this patch linger on without resolution isn't ideal either. The code in the patch shouldn't be prone to bugs since it's simply sending specially crafted UDP messages. I trust that Java sockets have been thoroughly tested and the xdr_ functions are very straight-forward (e.g. xdr_int is basically htonl()). Btw, your approach to sending the ganglia meta data each time is practical. It can be optimized in the future but that might add unwelcome complexity. There are a couple of options I can see.
1. Convert the test to be more of a unit test of GangliaContext31. So rather than use MiniDFSCluster, inject metrics directly into the context and check that they are correctly received by the socket listener. Ideally we'd do both, since 1 is a unit test for Ganglia, and 2 tests the integration of metrics and HDFS. Hey Tom,
I'm a bit confused on what to do next. Are you suggesting that we mark this as WONT FIX and re-open for HDFS? It's been about 9 months since I submitted the patch, so I'm mentally a bit behind. Brian
No, I think writing a unit test for GangliaContext31 (that doesn't use MiniDFSCluster) is needed to commit this. At a later point we could then follow up with a separate HDFS issue that has the test using MiniDFSCluster. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Open
Major
Current Ganglia metrics implementation is incompatible with Ganglia 3.1
1) Implement more XDR serializations. 3.0.x protocol sends all data as strings; 3.1.x correctly sends float data as XDR floats.
2) Test and verify sending Longs as floats in Ganglia actually works. This has been discussed, but I'm not sure it's entirely functional... maybe get a test case going?
3) Probably this will result in a new GangliaContext31 class, as I don't see any clean way to merge the two protocols together into one class.
HADOOP-3422caused a new metric name to be generated in the emitRecord class; we could reuse this if it was separated out into a new function.Brian