Re: Template configuration parameters with comma-separated lists in Janusgraph 0.6.0


Mladen Marović
 

Hello,

I ran the test CQLConfiguredGraphFactoryTest.templateConfigurationShouldSupportMultiHosts() and it was indeed successful. However, the test itself does not check the every-day scenario where it should be normal for Janusgraph instances to restart, graphs to be closed, etc. Let me demonstrate.

The test contains the following lines:

ConfiguredGraphFactory.createTemplateConfiguration(getTemplateConfigWithMultiHosts());
final StandardJanusGraph graph = (StandardJanusGraph) ConfiguredGraphFactory.create("graph1");
final StandardJanusGraph graph1 = (StandardJanusGraph) ConfiguredGraphFactory.open("graph1");
assertNotNull(graph);
assertEquals(graph, graph1);

ConfiguredGraphFactory.create() fetches a configuration template, fills some additional properties, opens the graph using this new configuration object (very important), and then stores the configuration object using:

configManagementGraph.createConfiguration(ConfigurationUtil.loadMapConfiguration(templateConfigMap));  // Very suspicious!!!

The next line in the test, ConfiguredGraphFactory.open(), attempts to open that graph, but since it was already opened in the previous create() call, JanusGraphManager will simply return the already successfully opened instance.

However, it seems to me that the problem appears in the configuration saved at the end of the create() call (the suspicious line) because it forces list parsing. Therefore, the stored configuration is different from the one used to open the graph for the first time, but it will be used in all other open() calls, e.g. in my case after restarting the Janusgraph instance. To prove this, I changed the test only slightly, to close the graph before opening it again:

ConfiguredGraphFactory.createTemplateConfiguration(getTemplateConfigWithMultiHosts());
final StandardJanusGraph graph = (StandardJanusGraph) ConfiguredGraphFactory.create("graph1");
graph.close();
final StandardJanusGraph graph1 = (StandardJanusGraph) ConfiguredGraphFactory.open("graph1");
// assertNotNull(graph);
// assertEquals(graph, graph1);
assertNotNull(graph1);

This test crashes when trying to open graph1 after the close().

Since list parsing is forced when opening the graph both as part of the create() call and in the open() call, the configuration object at the end of the create() call should be stored as is, without any changes. After changing the suspicious line to:

configManagementGraph.createConfiguration(new MapConfiguration(templateConfigMap));

the test passed without issues.

I haven't tried building a custom Janusgraph release to test this change fully in my case yet, but I suspect that it should be OK. If someone else can confirm that this is indeed a bug, I'll be happy to open an issue on GitHub and submit a PR.

Kind regards,

Mladen Marović

Join janusgraph-users@lists.lfaidata.foundation to automatically receive all group messages.