----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25971/#review57157 -----------------------------------------------------------
giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/Server.java <https://reviews.apache.org/r/25971/#comment97661> Will it be replaced with something real? giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/Server.java <https://reviews.apache.org/r/25971/#comment97659> Is it some custom tag library? Maybe you want annotation instead? I'm not sure what it means giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerHttpHandler.java <https://reviews.apache.org/r/25971/#comment97641> I'm not sure about our policy on using oracle specific features. How hard it would be to switch to jetty? We already have it in classpath anyway. giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerHttpHandler.java <https://reviews.apache.org/r/25971/#comment97652> Why would you need it? giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerHttpHandler.java <https://reviews.apache.org/r/25971/#comment97654> Can you please use standard javadoc tags? giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerHttpHandler.java <https://reviews.apache.org/r/25971/#comment97658> NumberFormatException extends IllegalArgumentException so this condition is always false giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/AbstractInterceptingComputation.java <https://reviews.apache.org/r/25971/#comment97633> These fields are not constants, so should not be named with uppercase letters. And I'm not sure why they made static too. Is it necessary? giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/InstrumentGiraphClasses.java <https://reviews.apache.org/r/25971/#comment97684> Just createTempDirectory(TMP_DIR_NAME_PREFIX) will work giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/InstrumentGiraphClasses.java <https://reviews.apache.org/r/25971/#comment97681> Do e.printStackTrace() at least. We should know which classes were not found. giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/InstrumentGiraphClasses.java <https://reviews.apache.org/r/25971/#comment97686> Either mc or masterComputeClass is redundand giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/InstrumentGiraphClasses.java <https://reviews.apache.org/r/25971/#comment97679> targetTopClass is always CtClass, how can this equals be satisfied? Did you mean targetTopClass.toClass().equals(Object.class) ? giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/BaseWrapper.java <https://reviews.apache.org/r/25971/#comment97663> just method.invoke(urlClassLoader, u); will work fine This diff is huge and takes a lot of time to comprehend, I focussed on code quality issues now and will need another pass for more general issues. - Sergey Edunov On Sept. 24, 2014, 1:07 a.m., Jaeho Shin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25971/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2014, 1:07 a.m.) > > > Review request for giraph and Sergey Edunov. > > > Bugs: GIRAPH-905 > https://issues.apache.org/jira/browse/GIRAPH-905 > > > Repository: giraph-git > > > Description > ------- > > A rather large patch to include Giraph Debugger (Graft) in Giraph trunk. > Based on [222e5e of > semihsalihoglu/graft](https://github.com/semihsalihoglu/graft/commit/222e5e7c86c3c4122c1b88336b17966d4e7728d0). > > > Diffs > ----- > > giraph-core/src/main/java/org/apache/giraph/master/MasterCompute.java > 552cca9 > giraph-debugger/.gitignore PRE-CREATION > giraph-debugger/README.md PRE-CREATION > giraph-debugger/giraph-debug PRE-CREATION > giraph-debugger/gui PRE-CREATION > giraph-debugger/pom.xml PRE-CREATION > giraph-debugger/src/main/assembly/compile.xml PRE-CREATION > giraph-debugger/src/main/java/org/apache/giraph/debugger/CommandLine.java > PRE-CREATION > giraph-debugger/src/main/java/org/apache/giraph/debugger/DebugConfig.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/exceptiondebug/BuggySimpleTriangleClosingComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/exceptiondebug/SimpleTriangleClosingDebugConfig.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/exceptiondebug/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggyConnectedComponentsDebugComputationModified.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggyConnectedComponentsDebugComputationToRun.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggySimpleShortestPathsDebugComputationModified.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggySimpleShortestPathsDebugComputationToRun.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggySimpleTriangleClosingDebugComputationModified.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/BuggySimpleTriangleClosingDebugComputationToRun.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/instrumented/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/integrity/BuggyConnectedComponentsComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/integrity/ConnectedComponentsMsgIntegrityDebugConfig.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/integrity/ConnectedComponentsVValueIntegrityDebugConfig.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/integrity/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/simpledebug/BuggySimpleShortestPathsComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/simpledebug/SimpleShortestPathsDebugConfig.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/simpledebug/SimpleShortestPathsMaster.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/examples/simpledebug/package-info.java > PRE-CREATION > giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/Server.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerHttpHandler.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/ServerUtils.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/gui/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/AbstractInterceptingComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/AbstractInterceptingMasterCompute.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/BottomInterceptingComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/BottomInterceptingMasterCompute.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/CommonVertexMasterInterceptionUtil.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/InstrumentGiraphClasses.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/Intercept.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/UserComputation.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/UserMasterCompute.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/instrumenter/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/ComputationComputeTestGenerator.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/FormatHelper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/MasterComputeTestGenerator.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/PrefixedClasspathResourceLoader.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/TestGenerator.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/TestGraphGenerator.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/VelocityBasedGenerator.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/mock/package-info.java > PRE-CREATION > giraph-debugger/src/main/java/org/apache/giraph/debugger/package-info.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/AggregatedValueWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/AggregatorWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/BaseScenarioAndIntegrityWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/BaseWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/CommonVertexMasterContextWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/DebuggerUtils.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/ExceptionWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/GiraphMasterScenarioWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/GiraphVertexScenarioWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/MsgIntegrityViolationWrapper.java > PRE-CREATION > > giraph-debugger/src/main/java/org/apache/giraph/debugger/utils/package-info.java > PRE-CREATION > giraph-debugger/src/main/protobuf/giraph_aggregator.proto PRE-CREATION > giraph-debugger/src/main/protobuf/integrity.proto PRE-CREATION > giraph-debugger/src/main/protobuf/scenario.proto PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/css/app.css > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/css/slider/slider.css > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/css/valpanel.css > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/img/details_close.png > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/img/details_open.png > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/img/preloader.gif > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/index.html > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/debugger.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/editor.core.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/editor.utils.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/slider/bootstrap-slider.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/utils.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/utils.sampleGraphs.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/gui/js/valpanel.js > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/ComputeSetUpFuncTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/ComputeTestFuncTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/ComputeTestTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/MasterComputeTestTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/ReadWritableFromByteArrayTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/ReadWritableFromStringTemplate.vm > PRE-CREATION > > giraph-debugger/src/main/resources/org/apache/giraph/debugger/mock/TestGraphTemplate.vm > PRE-CREATION > > giraph-debugger/src/test/java/org/apache/giraph/debugger/instrumenter/test/basecompute/BaseComputation.java > PRE-CREATION > > giraph-debugger/src/test/java/org/apache/giraph/debugger/instrumenter/test/basecompute/CommonDebugConfig.java > PRE-CREATION > > giraph-debugger/src/test/java/org/apache/giraph/debugger/instrumenter/test/basecompute/DerivedComputation.java > PRE-CREATION > > giraph-debugger/src/test/java/org/apache/giraph/debugger/instrumenter/test/basecompute/package-info.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25971/diff/ > > > Testing > ------- > > Manually followed the steps for debugging shown in the > [giraph-debugger/README.md](https://github.com/semihsalihoglu/graft#launching-giraph-jobs-with-graft), > and confirmed generated test codes for Computation.compute() and > MasterCompute with GUI and CLI reproduces the context. > > For launching a buggy example (included under giraph-debugger), used the > following command: > ``` > ../giraph-debugger/giraph-debug > ../giraph-debugger/target/giraph-debugger-1.1.0-SNAPSHOT-for-hadoop-1.2.1-jar-with-dependencies.jar > org.apache.giraph.GiraphRunner > org.apache.giraph.debugger.examples.simpledebug.BuggySimpleShortestPathsComputation > -mc > org.apache.giraph.debugger.examples.simpledebug.SimpleShortestPathsMaster > -vif org.apache.giraph.io.formats.JsonLongDoubleFloatDoubleVertexInputFormat > -vip shortestPathsInputGraph -vof > org.apache.giraph.io.formats.IdWithValueTextOutputFormat -op > shortestPathsOutputGraph.$RANDOM -w 1 -ca > giraph.SplitMasterWorker=false # > ``` > > > Thanks, > > Jaeho Shin > >
