Merge pull request JanusGraph#445 from smcquillan/titanIdStoreIssue418

Failure to correctly set ids.store-name=titan_ids can corrupt a Titan Cassandra Issue JanusGraph#418

Author: jerryjch

docs/advanced.txt

@@ -20,3 +20,5 @@ include::serializer.txt[]
 include::hadoop.txt[]
 
 include::monitoring.txt[]
+
+include::migrating.txt[]

docs/migrating.txt

@@ -0,0 +1,43 @@
+[[migrating-titan]]
+== Migrating from Titan
+
+This page describes some of the Configuration options that JanusGraph provides to allow migration of data from a data store which had previously been created by Titan.
+
+=== Configuration
+
+When connecting to an existing Titan data store the `graph.titan-version` property should be set to the Titan version used to create that store.
+
+[source, properties]
+----
+graph.titan-version=1.0.0
+----
+
+The ID store name in JanusGraph is configurable via the `ids.store-name` property whereas in Titan it was a constant. If the `graph.titan-version` has been set, as in the example above, then you do **not** need to explicitly set this value as it will default to `titan_ids`.
+
+=== Cassandra
+
+The default keyspace used by Titan was `titan` and in order to reuse that existing keyspace the `storage.cassandra.keyspace` property needs to be set accordingly.
+
+[source, properties]
+----
+graph.titan-version=1.0.0
+storage.cassandra.keyspace=titan
+----
+
+These configuration options allow JanusGraph to read data from a Cassandra database which had previously been created by Titan. However, once JanusGraph writes back to that database it will register additional serializers which mean that it will no longer be compatible with Titan. Users are therefore encouraged to backup the data in Casssandra before attempting to use it with the JanusGraph release. 
+
+=== HBase
+
+The name of the table used by Titan was `titan` and in order to reuse that existing table the `storage.hbase.table` property needs to be set accordingly.
+
+[source, properties]
+----
+graph.titan-version=1.0.0
+storage.hbase.table=titan
+----
+
+These configuration options allow JanusGraph to read data from an HBase database which had previously been created by Titan. However, once JanusGraph writes back to that database it will register additional serializers which mean that it will no longer be compatible with Titan. Users are therefore encouraged to backup the data in HBase before attempting to use it with the JanusGraph release. 
+
+=== BerkeleyDB
+
+The BerkeleyDB version has been updated, and it contains changes to the file format stored on disk. This file format change is forward compatible with previous versions of BerkeleyDB, so existing graph data stored with Titan can be read in. However, once the data has been read in with the newer version of BerkeleyDB, those files can no longer be read by the older version. Users are encouraged to backup the BerkeleyDB storage directory before attempting to use it with the JanusGraph release.

docs/upgrade.txt

@@ -19,15 +19,7 @@ your code and configuration accordingly:
   duplicate a word, e.g., `TitanGraph` is simply `JanusGraph` rather than
   `JanusGraphGraph`
 
-*IMPORTANT* If you are pointing JanusGraph at an existing Titan database you will need to
-set the `ids.store-name` property to `titan_ids` in your JanusGraph properties file.
-Failure to do this could result in id allocation issues.
-See https://github.com/JanusGraph/janusgraph/issues/228[228] for more details.
-
-The BerkeleyDB version has been updated, and it contains changes to the file
-format stored on disk. This file format change is forward compatible with
-previous versions of BerkeleyDB, so existing graph data stored with Titan
-can be read in. However, once the data has been read in with the newer version
-of BerkeleyDB, those files can no longer be read by the older version. Users
-are encouraged to backup the BerkeleyDB storage directory before attempting to
-use it with the JanusGraph release.
+*IMPORTANT* If you are pointing JanusGraph at an existing Titan database you will
+need to set the `graph.titan-version` property. For more information on how to
+configure JanusGraph to read data which had previously been written by Titan
+refer to <<migrating-titan>>.

janusgraph-cassandra/src/test/java/org/janusgraph/graphdb/CassandraGraphTest.java

@@ -128,7 +128,12 @@ public void testTitanGraphBackwardCompatibility() {
                             wc.get(ConfigElement.getPath(GraphDatabaseConfiguration.TITAN_COMPATIBLE_VERSIONS), 
                                         GraphDatabaseConfiguration.TITAN_COMPATIBLE_VERSIONS.getDatatype())
                 ));
-
+        
+        wc.set(ConfigElement.getPath(GraphDatabaseConfiguration.IDS_STORE_NAME), JanusGraphConstants.TITAN_ID_STORE_NAME);
+        assertTrue(JanusGraphConstants.TITAN_ID_STORE_NAME.equals(
+                            wc.get(ConfigElement.getPath(GraphDatabaseConfiguration.IDS_STORE_NAME), 
+                                        GraphDatabaseConfiguration.IDS_STORE_NAME.getDatatype())
+                ));
         graph = (StandardJanusGraph) JanusGraphFactory.open(wc);
     }
 }

janusgraph-core/src/main/java/org/janusgraph/graphdb/configuration/GraphDatabaseConfiguration.java

@@ -724,7 +724,7 @@ public boolean apply(@Nullable Integer integer) {
     public static final ConfigOption<String> IDS_STORE_NAME = new ConfigOption<>(IDS_NS, "store-name",
         "The name of the ID KCVStore. IDS_STORE_NAME is meant to be used only for backward compatibility with Titan, " +
             "and should not be used explicitly in normal operations or in new graphs.",
-        ConfigOption.Type.GLOBAL_OFFLINE, "janusgraph_ids");
+        ConfigOption.Type.GLOBAL_OFFLINE, JanusGraphConstants.JANUSGRAPH_ID_STORE_NAME);
 
 
     /**
@@ -1402,7 +1402,7 @@ public GraphDatabaseConfiguration(ReadConfiguration localConfig) {
                         throw new JanusGraphException(String.format(INCOMPATIBLE_VERSION_EXCEPTION, version, JanusGraphConstants.VERSION));
                     }
                 } catch (IllegalStateException ise) {
-                    checkBackwardCompatibilityWithTitan(globalWrite);
+                    checkBackwardCompatibilityWithTitan(globalWrite, localbc, kcvsConfig, overwrite);
                 }
 
                 final boolean managedOverridesAllowed;
@@ -1426,6 +1426,12 @@ else if (globalWrite.has(ALLOW_STALE_CONFIG))
                     // Get the storage backend's setting and compare with localValue
                     Object storeValue = globalWrite.get(opt, pid.umbrellaElements);
 
+                    // Check if the value is to be overwritten
+                    if (overwrite.has(opt, pid.umbrellaElements))
+                    {
+                    	storeValue = overwrite.get(opt, pid.umbrellaElements);
+                    }
+
                     // Most validation predicate impls disallow null, but we can't assume that here
                     final boolean match;
                     if (null != localValue && null != storeValue) {
@@ -1493,12 +1499,28 @@ else if (globalWrite.has(ALLOW_STALE_CONFIG))
         preLoadConfiguration();
     }
 
-    private void checkBackwardCompatibilityWithTitan(ModifiableConfiguration globalWrite) {
+    private void checkBackwardCompatibilityWithTitan(ModifiableConfiguration globalWrite, BasicConfiguration localbc, KCVSConfiguration kcvsConfig, ModifiableConfiguration overwrite) {
         String version = globalWrite.get(TITAN_COMPATIBLE_VERSIONS);
         Preconditions.checkArgument(version!=null,"JanusGraph version nor Titan compatibility have not been initialized");
         if (!JanusGraphConstants.TITAN_COMPATIBLE_VERSIONS.contains(version)) {
             throw new JanusGraphException(String.format(INCOMPATIBLE_VERSION_EXCEPTION, version, JanusGraphConstants.VERSION));
         }
+
+        // When connecting to a store created by Titan the ID store name will not be in the
+        // global configuration. To ensure compatibility override the default to titan_ids.
+        boolean localTitanConfigured = localbc.get(TITAN_COMPATIBLE_VERSIONS) != null;
+        boolean localIdStoreIsDefault = JanusGraphConstants.JANUSGRAPH_ID_STORE_NAME.equals(localbc.get(IDS_STORE_NAME));
+
+        if (localTitanConfigured == true) {
+            boolean usingTitanIdStore = localIdStoreIsDefault == true || JanusGraphConstants.TITAN_ID_STORE_NAME.equals(localbc.get(IDS_STORE_NAME));
+            boolean existingKeyStore = kcvsConfig.get(IDS_STORE_NAME.getName(), IDS_STORE_NAME.getDatatype()) != null;
+
+        	Preconditions.checkArgument(usingTitanIdStore,"ID store for Titan compatibility has not been initialized to: " + JanusGraphConstants.TITAN_ID_STORE_NAME);
+            if (existingKeyStore == false) {
+                log.info("Setting {} to {} for Titan compatibility", IDS_STORE_NAME.getName(), JanusGraphConstants.TITAN_ID_STORE_NAME);
+                overwrite.set(IDS_STORE_NAME, JanusGraphConstants.TITAN_ID_STORE_NAME);
+            }
+        }
     }
 
     private static Map<ConfigElement.PathIdentifier, Object> getGlobalSubset(Map<ConfigElement.PathIdentifier, Object> m) {

janusgraph-core/src/main/java/org/janusgraph/graphdb/configuration/JanusGraphConstants.java

@@ -50,6 +50,16 @@ public class JanusGraphConstants {
      */
     public static final List<String> TITAN_COMPATIBLE_VERSIONS;
 
+    /**
+     * Name of the ids.store-name used by JanusGraph which is configurable
+     */
+    public static final String JANUSGRAPH_ID_STORE_NAME = "janusgraph_ids";
+
+    /**
+     * Past name of the ids.store-name used by Titan Graph but which was not configurable
+     */
+    public static final String TITAN_ID_STORE_NAME = "titan_ids";
+
     static {
 
         /*