storage.hostname and index.[x].hostname don't work with multiple hostnames

[FlorianHockmann]

It seems to be no longer possible to configure multiple hostnames for the storage or index backend in version 0.6.0.

For confirmed bugs, please report:

• Version: 0.6.0
• Storage Backend: CQL
• Mixed Index Backend: elasticsearch
• Link to discussed bug: -
• Expected Behavior: Add all hostnames as individual contact points
• Current Behavior: The hostnames are treated as a single hostname which results in this WARN log:

  WARN  com.datastax.oss.driver.internal.core.ContactPoints  - Ignoring invalid contact point testhost1,testhost2:9042 (unknown host testhost1,testhost2)

  The same happens for index.[x].hostname with elasticsearch.

• Steps to Reproduce: Start JanusGraph Server with a config .properties file that contains a list of hostnames for storage.cql.hostname or index.[x].hostname.

storage-cql-es.properties:

storage.hostname=testhost1,testhost2
index.search.hostname=testhost1,testhost2

This bug must have been introduced in 0.6.0 as it used to work in version 0.5.3.

[sergeymetallic]

If I start a gremlin server with a property in gremlin-server.yaml graphManager: "org.janusgraph.graphdb.management.JanusGraphManager" it fixes the problem. So the root of the problem seems to be in how Gremlin reads the settings and passes them to Janusgraph in org.apache.tinkerpop.gremlin.server.util.DefaultGraphManager

[FlorianHockmann]

Thanks, @sergeymetallic, this seems to work! I wonder whether we need to add this to default YAML config files that are shipped with JanusGraph or whether this needs to be fixed in a different place. @farodin91 Do you maybe see what's exactly the issue here?

Another thing I just noticed when starting JanusGraph Server with this config is that I'm getting this log line which also doesn't sound that good:

INFO  org.apache.commons.beanutils.FluentPropertyBeanIntrospector  - Error when creating PropertyDescriptor for public final void org.apache.commons.configuration2.AbstractConfiguration.setProperty(java.lang.String,java.lang.Object)! Ignoring this property.

So one property setting is completely ignored?

[farodin91]

@FlorianHockmann So one property setting is completely ignored?

Do you get this with JanusGraphManager, with DefaultGraphManager, or with both?

[farodin91]

I think the major between both GraphManager is that both call a different function DefaultGraphManager calls GraphFactory and JanusGraphManager calls JanusGraphFactory.

We could use JanusGraphManager in the most case, before we should add some tests for it.

[vtslab]

Possibly related: https://lists.lfaidata.foundation/g/janusgraph-users/topic/template_configuration/85766873?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,85766873,previd=1632837537479036875,nextid=1629862232378717843&previd=1632837537479036875&nextid=1629862232378717843

[FlorianHockmann]

Do you get this with JanusGraphManager, with DefaultGraphManager, or with both?

Only with JanusGraphManager.

[li-boxuan]

@FlorianHockmann Can you check if this issue goes away after https://github.com/JanusGraph/janusgraph/pull/2834 fix?

[FlorianHockmann]

@li-boxuan Doesn't #2834 only affect the ConfiguredGraphFactory? We're not using that. But just to be sure, I just tried it with the zip archive built by GitHub Actions in that PR and I'm still seeing the problem where a comma separated list of hostnames is treated as a single hostname.

[li-boxuan]

I don't think there is much JanusGraph can do if you use DefaultGraphManager from TinkerPop. GraphFactory from TinkerPop uses DisabledListDelimiterHandler but JanusGraphFactory uses comma-delimited DefaultListDelimiterHandler. The solution is to use JanusGraphManager which uses JanusGraphFactory.

It seems the root cause is due to the breaking change in TinkerPop, specifically, this commit: https://github.com/apache/tinkerpop/commit/607224a92e443f712ca86b4b38e2fe9f92e405dc In apache configuration, the comma-separated delimiter is used by default, while in apache configuration2, no delimiter is used by default. Created https://issues.apache.org/jira/browse/TINKERPOP-2629.

[FlorianHockmann]

The solution is to use JanusGraphManager which uses JanusGraphFactory.

OK, but then we should make that the default for JanusGraph Server in my opinion. Otherwise, comma separated hostnames won't work out of the box . But that leads me of course back to the log message I mentioned above about an ignored property that only happens with the JanusGraphManager.

[sergeymetallic]

The solution is to use JanusGraphManager which uses JanusGraphFactory.

OK, but then we should make that the default for JanusGraph Server in my opinion. Otherwise, comma separated hostnames won't work out of the box . But that leads me of course back to the log message I mentioned above about an ignored property that only happens with the JanusGraphManager.

The warning that you see happens in https://github.com/apache/commons-beanutils/blob/master/src/main/java/org/apache/commons/beanutils2/FluentPropertyBeanIntrospector.java#L135 Where "setProperty" in AbstractConfiguration is detected as bean setter, but in reality it is not (real setter should have only one parameter). This is not an issue, just a bad method naming for a bean

[FlorianHockmann]

Thanks for looking into this, @sergeymetallic! Good to know that this is really harmless although I would of course prefer if we wouldn't produce this log message on every startup but looks like we have to live with it for now. Then I guess that we can really close this issue by simply making JanusGraphManager the default for JanusGraph Server. Or are there any good reasons why we shouldn't do that?

[farodin91]

We should compare JanusGraphManager, if we see a difference. For example: what happens if a user needs tinkergraph at the same time with Janusgraph

[FlorianHockmann]

We should compare JanusGraphManager, if we see a difference. For example: what happens if a user needs tinkergraph at the same time with Janusgraph

I just checked and serialization of a TinkerGraph isn't supported out of the box, irrespective of the graph manager. I think that we should change that so I created an issue for that: #2843. If the serialization is configured correctly however, then it works also with the JanusGraphManager.

I also verified that Gremlin Server config values still work like gremlinPool or maxContentLength. Not sure what we need to verify else.