Hi Boxuan,

thanks for the confirmation, no need to apologize. I created the related issue at https://github.com/JanusGraph/janusgraph/issues/2833 and will submit a pull request after some more testing.

Kind regards,

Mladen Marović

Hi Mladen,

I can confirm this is indeed a bug. I apologize that when writing the test, I didn't notice the second open call was just returning the cached instance. Thank you for reporting and fixing it! It would be great if you could create an issue and a pull request for it.

Best regards,
Boxuan

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ć

Hi Mladen,

You are right, your issue is different than the one I mentioned about GraphManager.

I mentioned earlier that the JanusGraph test suite covers your use case:
https://github.com/JanusGraph/janusgraph/blob/v0.6.0/janusgraph-cql/src/test/java/org/janusgraph/core/cql/CQLConfiguredGraphFactoryTest.java

You can verify in the CI logs that this test passes successfully, see e.g. for a recent commit on master (the line starting with "Run mvn verify"):
https://github.com/JanusGraph/janusgraph/runs/3775465848?check_suite_focus=true

I do acknowledge that the return value you mention for

ConfiguredGraphFactory.getConfiguration('default_').get('storage.hostname')
==>[test-master, test-worker1]

look suspicious, but in what way does your use case differ from the tests with "MultiHost" CQL?

Best wishes,    Marc

Hi Marc,

thanks for the reply, but the error in the linked issue is a bit different, and also I'm already using graphManager: "org.janusgraph.graphdb.management.JanusGraphManager", which seemed to fix the linked issue.

I found a somewhat hackish workaround that seems to create the graph properly. Instead of using:

ConfiguredGraphFactory.create('default_');

, I manually create a configuration from the template and open the graph:

// ConfiguredGraphFactory.create('default_');
map = new HashMap<String, Object>();
map.put('graph.graphname', 'default_');
ConfiguredGraphFactory.getTemplateConfiguration().each{k, v -> map.putIfAbsent(k, v)};
conf = new MapConfiguration(map);
ConfiguredGraphFactory.createConfiguration(conf);
ConfiguredGraphFactory.open('default_');

After doing this, the graph is created seemingly without errors and I can use it as before.

I would expect ConfiguredGraphFactory.create() to do more or less exactly what I did manually, but the results were different and incorrect. Could there be some changes in ConfiguredGraphFactory.create() that cause it to behave differently than it did before?

Kind regards,

Mladen Marović

Hi Mladen,

After your original post, the following issue was reported:

https://github.com/JanusGraph/janusgraph/issues/2822

Marc

Hi,

thanks for the replies.

However, my point was that I'm explicitly setting a value WITHOUT brackets when creating a template configuration in the startup script:

map.put('storage.hostname', 'test-master,test-worker1');

, which also shows when I inspect that template configuration:

gremlin> ConfiguredGraphFactory.getTemplateConfiguration()
...
==>storage.hostname=test-master,test-worker1
...

, but when I create a new graph, it is created using a value WITH brackets:

gremlin> ConfiguredGraphFactory.getConfiguration('default_').get('storage.hostname')
==>[test-master, test-worker1]

which then produces the error when trying to open the graph.

To reiterate, I'm using a very similar script in 0.5.3 which works without any issues, but in 0.5.3 I disable list parsing manually when creating the template. In 0.6.0 the codebase switched to commons-configuration2 which has list parsing disabled by default, so I don't think I should have to disable it again. Even if I do, or use a different delimiter, it still doesn't work because creating the template configuration seems to work fine, but creating a new graph from that configuration does not.

I hope the issue itself is more clear now.

Kind regards,

Mladen Marović

Hi Mladen,

As Marc pointed out, this scenario is tested in the JanusGraph codebase. I noticed your error log contains:

2021-09-21 14:11:15,347 [pool-12-thread-1] com.datastax.oss.driver.internal.core.ContactPoints - WARN  - Ignoring invalid contact point [test-master:9042 (unknown host [test-master)
2021-09-21 14:11:15,348 [pool-12-thread-1] com.datastax.oss.driver.internal.core.ContactPoints - WARN  - Ignoring invalid contact point test-worker1]:9042 (unknown host test-worker1])

It seems that you have redundant brackets around the value. Try using "test-master, test-worker1" rather than "[test-master, test-worker1]".

Let me know if this helps.
Best, Boxuan

Hi Mladen,

The good news: there actually is an explicit MultiHost test for this in the janusgraph test suite:
https://github.com/JanusGraph/janusgraph/blob/v0.6.0/janusgraph-cql/src/test/java/org/janusgraph/core/cql/CQLConfiguredGraphFactoryTest.java

The bad news:  I could not get this test to run in my local environment. When I do (requires the docker service running):
$ git checkout v.0.6.0
$ mvn clean install -DskipTests
$ mvn test -Dtest=CQLConfiguredGraphFactoryTest -pl janusgraph-cql -Pcassandra3-murmur -e

all 21 tests fail with:

Caused by: org.janusgraph.graphdb.management.utils.ConfigurationManagementGraphNotEnabledException: Please add a key named "ConfigurationManagementGraph" to the "graphs" property in your YAML file and restart the server to be able to use the functionality of the ConfigurationManagementGraph class.

As far as I can see, the tests are run on github CI, see: https://github.com/JanusGraph/janusgraph/blob/master/.github/workflows/ci-backend-cql.yml
However, I cannot access the CI logs right now.

I do not know where to go from here.

Best wishes,    Marc

On Tue, Sep 21, 2021 at 02:29 PM, Mladen Marović wrote:

ConfiguredGraphFactory

Hello,

I wanted to try out the new Janusgraph 0.6.0 release and I encountered some unexpected issues while trying to deploy it on Cassandra 3.11.5.

One of the issues I came across seems to be connected to the switch to commons-configuration2. In previous Janusgraph versions, I used a startup script to create a configuration template for graphs and a single default_ graph:

def globals = [:]

globals << [hook : [
    onStartUp: { ctx ->
        def map = new HashMap<String, Object>();

        map.put('schema.default', 'none');
        map.put('schema.constraints', 'true');

        map.put('graph.allow-upgrade', 'false');

        map.put('storage.backend', 'cql');
        map.put('storage.hostname', 'test-master,test-worker1');
        map.put('storage.cql.replication-factor', '1');
        map.put('storage.cql.read-consistency-level', 'LOCAL_QUORUM');
        map.put('storage.cql.write-consistency-level', 'LOCAL_QUORUM');
        map.put('storage.cql.only-use-local-consistency-for-system-operations', 'true');
        map.put('storage.cql.local-datacenter', 'dc1');
        map.put('storage.cql.replication-strategy-options', 'dc1,1');
        map.put('storage.cql.replication-strategy-class', 'NetworkTopologyStrategy');

        map.put('index.es.backend', 'elasticsearch');
        map.put('index.es.hostname', 'test-dev-elasticsearch');
        map.put('index.es.elasticsearch.client-only', "true");
        map.put('index.es.elasticsearch.create.ext.index.number_of_shards', '5');
        map.put('index.es.elasticsearch.create.ext.index.number_of_replicas', '1');

        def conf = new MapConfiguration(map);
        conf.setDelimiterParsingDisabled(true);

        if (ConfiguredGraphFactory.getTemplateConfiguration() == null) {
            ConfiguredGraphFactory.createTemplateConfiguration(conf);
            ctx.logger.info("Successfully created the template configuration");
        } else {
            ConfiguredGraphFactory.updateTemplateConfiguration(conf);
            ctx.logger.info("Successfully updated the template configuration");
        }

        if (ConfiguredGraphFactory.getConfiguration('default_') == null) {
            ConfiguredGraphFactory.create('default_');
            ctx.logger.info("Successfully created the graph 'default_'");
        }
    }
] as LifeCycleHook]

An important piece of this was the line conf.setDelimiterParsingDisabled(true); which was a workaround to disable automatically parsing multiple comma-separated hostnames and other similar parameters as lists and keep them as strings. In commons-configuration2 this method does not exist and the docs (https://commons.apache.org/proper/commons-configuration/userguide/upgradeto2_0.html, https://commons.apache.org/proper/commons-configuration/apidocs/org/apache/commons/configuration2/MapConfiguration.html) state that list splitting is disabled by default. This is also confirmed here: https://github.com/JanusGraph/janusgraph/issues/1447#issuecomment-851119479.

After installing Janusgraph 0.6.0 I used a similar script, but with conf.setDelimiterParsingDisabled(true); commented out:

def globals = [:]

globals << [hook : [
    onStartUp: { ctx ->
        def map = new HashMap<String, Object>();

        map.put('schema.default', 'none');
        map.put('schema.constraints', 'true');

        map.put('graph.allow-upgrade', 'false');

        map.put('storage.backend', 'cql');
        map.put('storage.hostname', 'test-master,test-worker1');
        map.put('storage.cql.replication-factor', '1');
        map.put('storage.cql.read-consistency-level', 'LOCAL_QUORUM');
        map.put('storage.cql.write-consistency-level', 'LOCAL_QUORUM');
        map.put('storage.cql.only-use-local-consistency-for-system-operations', 'true');
        map.put('storage.cql.local-datacenter', 'dc1');
        map.put('storage.cql.replication-strategy-options', 'dc1,1');
        map.put('storage.cql.replication-strategy-class', 'NetworkTopologyStrategy');
        map.put('storage.cql.local-max-connections-per-host', 5)
        map.put('storage.cql.max-requests-per-connection', 1024)
        map.put('storage.cql.executor-service.enabled', 'false')
        map.put('storage.parallel-backend-executor-service.core-pool-size', 100)

        map.put('index.es.backend', 'elasticsearch');
        map.put('index.es.hostname', 'test-dev-elasticsearch');
        map.put('index.es.elasticsearch.client-only', "true");
        map.put('index.es.elasticsearch.create.ext.index.number_of_shards', '5');
        map.put('index.es.elasticsearch.create.ext.index.number_of_replicas', '1');

        map.put('storage.cql.internal.string-configuration', 'datastax-java-driver { advanced.metadata.schema.debouncer.window = 1 second }');

        map.put('query.smart-limit', 'false')

        def conf = new MapConfiguration(map);
        // conf.setDelimiterParsingDisabled(true);

        if (ConfiguredGraphFactory.getTemplateConfiguration() == null) {
            ConfiguredGraphFactory.createTemplateConfiguration(conf);
            ctx.logger.info("Successfully created the template configuration");
        } else {
            ConfiguredGraphFactory.updateTemplateConfiguration(conf);
            ctx.logger.info("Successfully updated the template configuration");
        }

        if (ConfiguredGraphFactory.getConfiguration('default_') == null) {
            ConfiguredGraphFactory.create('default_');
            ctx.logger.info("Successfully created the graph 'default_'");
        }
    }
] as LifeCycleHook]

The script executed without errors, but the graph default_ cannot be opened. I get the following exception:

2021-09-21 14:11:15,347 [pool-12-thread-1] com.datastax.oss.driver.internal.core.ContactPoints - WARN  - Ignoring invalid contact point [test-master:9042 (unknown host [test-master)
2021-09-21 14:11:15,348 [pool-12-thread-1] com.datastax.oss.driver.internal.core.ContactPoints - WARN  - Ignoring invalid contact point test-worker1]:9042 (unknown host test-worker1])
2021-09-21 14:11:15,352 [JanusGraph Session-admin-0] com.datastax.oss.driver.internal.core.time.Clock - INFO  - Using native clock for microsecond precision
2021-09-21 14:11:15,352 [JanusGraph Session-admin-0] com.datastax.oss.driver.internal.core.metadata.MetadataManager - INFO  - [JanusGraph Session] No contact points provided, defaulting to /127.0.0.1:9042
2021-09-21 14:11:15,355 [JanusGraph Session-admin-1] com.datastax.oss.driver.internal.core.control.ControlConnection - WARN  - [JanusGraph Session] Error connecting to Node(endPoint=/127.0.0.1:9042, hostId=null, hashCode=74fef982), trying next node (ConnectionInitException: [JanusGraph Session|control|connecting...] Protocol initialization request, step 1 (OPTIONS): failed to send request (io.netty.channel.StacklessClosedChannelException))
2021-09-21 14:11:17,434 [pool-12-thread-1] org.janusgraph.graphdb.management.JanusGraphManager - ERROR - Failed to open graph default_ with the following error:
 java.lang.IllegalArgumentException: Could not instantiate implementation: org.janusgraph.diskstorage.cql.CQLStoreManager.
Thus, it and its traversal will not be bound on this server.

This indicates that the string was split somewhere along the way, and the resulting array [test-master, test-worker1] was stored as the property value. Inspecting the configuration in the gremlin console also points to this:

gremlin> ConfiguredGraphFactory.getConfiguration('default_').get('storage.hostname')
==>[test-master, test-worker1]
gremlin> 
gremlin> ConfiguredGraphFactory.getConfiguration('default_').get('storage.hostname').getClass()
==>class java.lang.String
gremlin> 

I even tried to manually set the delimiter in my script to be something other than ',' (with something like conf.setListDelimiterHandler(new org.apache.commons.configuration2.convert.DefaultListDelimiterHandler(';' as char));), but that didn't help either.

Did anyone else come across this issue? Did I miss something else in the changelog about this?

Kind regards,

Mladen Marović