[jira] [Updated] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-1.patch This still has a couple small problems, its just a placeholder. However its 99% done. I will finish up tomorrow. There isn't a lot to recommend this, except: 1. It was a ton of work 2. The approach I took was extremely consistent given what I had to work with. 3. Only the ugly underbelly that deals with Hadoop stuff still has to use the original Configuration. Application code should never have to see/touch it. This sets us up to begin making a "cleaner break" from Hadoop's configuration. When a module needs Hadoop settings, I left them where they were, accessed through GiraphConf, so that all the messy dependencies are right where we left them, ready to be addressed. Where the conf was being used for settings involving only GiraphJob or user properties, they were shifted entirely to the GiraphConf with getters/setters. This was a lot of work. I hope I can refine/adapt this to fit what we wanted here, I think as a transition away from showing users and application writers the Hadoop dependencies, it goes a long long way. More to follow... > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > } > /** >* Set the graph partitioner class (optional) >* >* @param graphPartitionerFactoryClass Determines how the graph is > partitioned >*/ > public final void setGraphPartitionerFactoryClass( > Class graphPartitionerFactoryClass) { > getConfiguration().setClass(GRAPH_PARTITIONER_FACTORY_CLASS, > graphPartitionerFactoryClass, > GraphPartitionerFactory.class); > } > /** >* Set the vertex resolver class (optional) >* >* @param vertexResolverClass Determines how vertex mutations are resolved >*/ > public final void setVertexResolverClass(Class vertexResolverClass) { > getConfiguration().setClass(VERTEX_RESOLVER_CLASS, > vertexResolverClass, > VertexResolver.class); > } > /** >* Set the worker context class (optional) >* >* @param workerContextClass Determines what code is executed on a each >*worker before and after each superstep and computation >*/ > public final void setWorkerContextClass(Class workerContextClass) { > getConfiguration().setClass(WORKER_CONTEXT_CLASS, > workerContextClass, > WorkerContext.class); > } > ...etc. -- 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: ht
[jira] [Updated] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-2.patch This works, and has a few important fixes from the last version. I still need to replace dependencies in some of the unit tests (remove GiraphJob ref's and replace Configuration calls with the new get/sets from GiraphConf) which I will do tonight hopefully. One major fix I'd like to make now that I know I've factored the important stuff into GiraphConf is the to go through the affected code and remove all reference to Configuration where I can by removing the override setConf(Configuration conf) with setConf(GiraphConf conf). i was able to do this already in those files with getConf due to Java's covariant return types, but for all classes in the codebase that don't just hold a Configuration object but also implement Configurable (I think) I can't do this yet. Perhaps I can wrap that inheritance from hadoop in something that does sefConf(GiraphConf conf) and then remove the casting. During the pruning stage I have done this ugly fix, just to get the code out into GiraphConf and figure out where everything lives (but this is a bad solution): ... somewhere in some (many) files that inherit from Configurable/Configuration ... public void setConf(Configuration conf) { this.conf = (GiraphConf)conf; } again, I will fix the tests and this I think will represent a working transitional version that at least gets the conf fully separated out while we figure out what direction to take to make it a bit cleaner? Avery, when you made this JIRA issue, what were you envisioning this to be? > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch, GIRAPH-214-2.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > } > /** >* Set the graph partitioner class (optional) >* >* @param graphPartitionerFactoryClass Determines how the graph is > partitioned >*/ > public final void setGraphPartitionerFactoryClass( > Class graphPartitionerFactoryClass) { > getConfiguration().setClass(GRAPH_PARTITIONER_FACTORY_CLASS, > graphPartitionerFactoryClass, > GraphPartitionerFactory.class); > } > /** >* Set the vertex resolver class (optional) >* >* @param vertexResolverClass Determines how vertex mutations are resolved >*/ > public final void setVertexResolverClass(Class vertexResolverClass) { > getConfiguration().setClass(VERTEX_RESOLVER_CLASS, > vertexResolverClass, > VertexResolver.class); > } > /** >* Set the worker context class (optional) >* >* @param workerContextClass Determines what code is executed on a each >*worker before and after each superstep a
[jira] [Updated] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-3.patch This is nearly done. Having a bit of trouble with some of the unit tests. Could be symptoms of mistakes made earlier in the process, or just some housekeeping. Will finish up ASAP. Again, this simply splits off all Giraph-related functionality into its own Configuration subclass and attempts to shield end-users (application authors) from seeing Configuration directly by providing getters/setters for Giraph-related settings in the GiraphConf class. The basic plumbing came out of GiraphJob (which is a lot smaller now.) I still think this will make a great first step to the messy job of deciding how to better structure the use of Configuration that more experienced folks than I can debate and decide. Feel free to take a look and provide feedback/instructions if you see something you don't like while I'm still actively working on this. Any advice about the issues in the tests is welcome too! > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch, GIRAPH-214-2.patch, > GIRAPH-214-3.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > } > /** >* Set the graph partitioner class (optional) >* >* @param graphPartitionerFactoryClass Determines how the graph is > partitioned >*/ > public final void setGraphPartitionerFactoryClass( > Class graphPartitionerFactoryClass) { > getConfiguration().setClass(GRAPH_PARTITIONER_FACTORY_CLASS, > graphPartitionerFactoryClass, > GraphPartitionerFactory.class); > } > /** >* Set the vertex resolver class (optional) >* >* @param vertexResolverClass Determines how vertex mutations are resolved >*/ > public final void setVertexResolverClass(Class vertexResolverClass) { > getConfiguration().setClass(VERTEX_RESOLVER_CLASS, > vertexResolverClass, > VertexResolver.class); > } > /** >* Set the worker context class (optional) >* >* @param workerContextClass Determines what code is executed on a each >*worker before and after each superstep and computation >*/ > public final void setWorkerContextClass(Class workerContextClass) { > getConfiguration().setClass(WORKER_CONTEXT_CLASS, > workerContextClass, > WorkerContext.class); > } > ...etc. -- 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] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-4.patch > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch, GIRAPH-214-2.patch, > GIRAPH-214-3.patch, GIRAPH-214-4.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > } > /** >* Set the graph partitioner class (optional) >* >* @param graphPartitionerFactoryClass Determines how the graph is > partitioned >*/ > public final void setGraphPartitionerFactoryClass( > Class graphPartitionerFactoryClass) { > getConfiguration().setClass(GRAPH_PARTITIONER_FACTORY_CLASS, > graphPartitionerFactoryClass, > GraphPartitionerFactory.class); > } > /** >* Set the vertex resolver class (optional) >* >* @param vertexResolverClass Determines how vertex mutations are resolved >*/ > public final void setVertexResolverClass(Class vertexResolverClass) { > getConfiguration().setClass(VERTEX_RESOLVER_CLASS, > vertexResolverClass, > VertexResolver.class); > } > /** >* Set the worker context class (optional) >* >* @param workerContextClass Determines what code is executed on a each >*worker before and after each superstep and computation >*/ > public final void setWorkerContextClass(Class workerContextClass) { > getConfiguration().setClass(WORKER_CONTEXT_CLASS, > workerContextClass, > WorkerContext.class); > } > ...etc. -- 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] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-5-option1.patch Hi. I am working on two options for this patch. This compiles but won't run, and here's why: When we create a GiraphJob, we place a Configuration into it. It is passed to Hadoop, which COPIES that Configuration (in a JobConf) and stores it. When we receive this Configuration back on the Giraph side, it is passed to us from the Mapper.Context or JobContext Hadoop holds. The Configuration instances these two Hadoop objects return to us via getConfiguration() are org.apache.mapred.JobConf in all the Giraph profiles. Even when I attempt to set GiraphConf to inherit from JobConf, no casting to GiraphConf is allowed at runtime when these JobConf's arrive at our doorstep as they are JobConf copies and not GiraphConf's at all any more. Option 2 (not finished yet) involves having GiraphConf own the Configuration object rather than inherit from it, and route calls to it. Its a better approach on the surface, but this involves many instantiations of GiraphConf to wrap all the different Configuration instances we receive from the Hadoop side at each entry point back into Giraph code. Given our memory issues, this seems like a step in the wrong direction just to add some convenience accessor/mutator methods to Giraph. Jakob made the point that it is not desirable to place all of our config definitions into GiraphConf directly since some are domain specific and others are application specific. He felt it was more clear to allow users to see the definitions in the local code where their user-set values are read from the job's Configuration. This means raw calls to getInt, getClass etc. as the application author needs to extract their custom values. Giving even the application layer this raw Configuration access from within GiraphConf without getters and setters seems to negate the whole purpose of GiraphConf again. Before I go further on this, I would really like to gather some opinions about whether it is practical to do this at all at this stage in Giraph's development. Perhaps this should wait for a future JIRA that refactors our coupling with the Hadoop framework before this can be implemented in a useful way? If there's a nice option 3 I have not thought of, please feel free to tell me your idea or attempt it, most of the grunt work is done for you in the "option1" patch here already, as is consistent (for now) with the current trunk. > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch, GIRAPH-214-2.patch, > GIRAPH-214-3.patch, GIRAPH-214-4.patch, GIRAPH-214-5-option1.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > }
[jira] [Updated] (GIRAPH-214) GiraphJob should have configuration split out of it to be cleaner (GiraphConf)
[ https://issues.apache.org/jira/browse/GIRAPH-214?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eli Reisman updated GIRAPH-214: --- Attachment: GIRAPH-214-6-option1.patch This is another more convincing attempt to force GiraphJob help our GiraphConf instance to survive the Giraph->Hadoop->Giraph transition intact without altering the current interactions between the frameworks. Still the same problem, will continue to consider options. Just putting up here for reference and for others to take a look at if they have some ideas. > GiraphJob should have configuration split out of it to be cleaner (GiraphConf) > -- > > Key: GIRAPH-214 > URL: https://issues.apache.org/jira/browse/GIRAPH-214 > Project: Giraph > Issue Type: Bug >Reporter: Avery Ching >Assignee: Eli Reisman >Priority: Minor > Attachments: GIRAPH-214-1.patch, GIRAPH-214-2.patch, > GIRAPH-214-3.patch, GIRAPH-214-4.patch, GIRAPH-214-5-option1.patch, > GIRAPH-214-6-option1.patch > > > Currently all the configuration for Giraph is part of GiraphJob, making > things messy for GiraphJob. > It would be better if we added a GiraphConf (similar to Hive) that is > responsible for handling configuration of the Job. > i.e. > public class GiraphJob extends Configuration > To simplify config, we should make get/set methods for as many of the > parameters as possible. > We are targeting configuration such as > /** >* Set the vertex class (required) >* >* @param vertexClass Runs vertex computation >*/ > public final void setVertexClass(Class vertexClass) { > getConfiguration().setClass(VERTEX_CLASS, vertexClass, BasicVertex.class); > } > /** >* Set the vertex input format class (required) >* >* @param vertexInputFormatClass Determines how graph is input >*/ > public final void setVertexInputFormatClass( > Class vertexInputFormatClass) { > getConfiguration().setClass(VERTEX_INPUT_FORMAT_CLASS, > vertexInputFormatClass, > VertexInputFormat.class); > } > /** >* Set the vertex output format class (optional) >* >* @param vertexOutputFormatClass Determines how graph is output >*/ > public final void setVertexOutputFormatClass( > Class vertexOutputFormatClass) { > getConfiguration().setClass(VERTEX_OUTPUT_FORMAT_CLASS, > vertexOutputFormatClass, > VertexOutputFormat.class); > } > /** >* Set the vertex combiner class (optional) >* >* @param vertexCombinerClass Determines how vertex messages are combined >*/ > public final void setVertexCombinerClass(Class vertexCombinerClass) { > getConfiguration().setClass(VERTEX_COMBINER_CLASS, > vertexCombinerClass, > VertexCombiner.class); > } > /** >* Set the graph partitioner class (optional) >* >* @param graphPartitionerFactoryClass Determines how the graph is > partitioned >*/ > public final void setGraphPartitionerFactoryClass( > Class graphPartitionerFactoryClass) { > getConfiguration().setClass(GRAPH_PARTITIONER_FACTORY_CLASS, > graphPartitionerFactoryClass, > GraphPartitionerFactory.class); > } > /** >* Set the vertex resolver class (optional) >* >* @param vertexResolverClass Determines how vertex mutations are resolved >*/ > public final void setVertexResolverClass(Class vertexResolverClass) { > getConfiguration().setClass(VERTEX_RESOLVER_CLASS, > vertexResolverClass, > VertexResolver.class); > } > /** >* Set the worker context class (optional) >* >* @param workerContextClass Determines what code is executed on a each >*worker before and after each superstep and computation >*/ > public final void setWorkerContextClass(Class workerContextClass) { > getConfiguration().setClass(WORKER_CONTEXT_CLASS, > workerContextClass, > WorkerContext.class); > } > ...etc. -- 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