----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66253/#review200259 -----------------------------------------------------------
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeParsers.java Lines 107 (patched) <https://reviews.apache.org/r/66253/#comment280933> These constants are defined in too many places. Please consolidate to a single defintion. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeParsers.java Lines 156 (patched) <https://reviews.apache.org/r/66253/#comment280932> These constants are defined in too many places. Please consolidate to a single defintion. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeProcessManager.java Lines 44 (patched) <https://reviews.apache.org/r/66253/#comment280931> For better readability, please add comments on what 'rGraph' and 'bGraph' represent. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessManager.java Lines 38 (patched) <https://reviews.apache.org/r/66253/#comment280930> what does 'bGraph' mean? graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/ReaderStatusManager.java Lines 92 (patched) <https://reviews.apache.org/r/66253/#comment280929> in line #84, END_TIME is set to Date. Here it is set to a long value. Please review and update. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/RelationshipTypeCache.java Lines 56 (patched) <https://reviews.apache.org/r/66253/#comment280928> relationship can be between: - inputTypeName and a superType of outputTypeName - outputTypeName and a superType of inputTypeName So, getSuperType() should be getTypeAndSuperTypes(). Please review. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/pc/WorkItemConsumer.java Lines 85 (patched) <https://reviews.apache.org/r/66253/#comment280926> It seems 'commitDuration' tracks the maxCommitTime taken by this consumer. In that case, please consider renaming. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/pc/WorkItemManager.java Lines 57 (patched) <https://reviews.apache.org/r/66253/#comment280927> avgDuration is in seconds; the wait is in minutes - intentional? Please review. graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeManagerTest.java Lines 20 (patched) <https://reviews.apache.org/r/66253/#comment280925> If this empty class is not used/needed, please remove. graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessManagerTest.java Lines 28 (patched) <https://reviews.apache.org/r/66253/#comment280924> If this empty class is not used/needed, please remove. repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java Lines 289 (patched) <https://reviews.apache.org/r/66253/#comment280922> VERTEX_ORIGINAL_ID_KEY => VERTEX_ID_IN_IMPORT_KEY repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java Lines 298 (patched) <https://reviews.apache.org/r/66253/#comment280923> EDGE_ORIGINAL_ID_KEY => EDGE_ID_IN_IMPORT_KEY repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java Lines 21 (patched) <https://reviews.apache.org/r/66253/#comment280920> I would suggest to avoid adding a new dependency - unless it is critical. repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java Lines 98 (patched) <https://reviews.apache.org/r/66253/#comment280921> Consider using: GraphHelper.getSingleValuedProperty(vertex, START_TIME_PROPERTY, Date.class) repository/src/main/java/org/apache/atlas/repository/migration/RelationshipCacheGenerator.java Lines 37 (patched) <https://reviews.apache.org/r/66253/#comment280919> this seems to assume that there can be only one relationship type between a given 2 entity-types. This is not true. For example, between hive_table and hive_column types, following relationship types exist: - hive_table_columns - hive_table_partitionkeys Please review. repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java Lines 95 (patched) <https://reviews.apache.org/r/66253/#comment280917> When migration is enabled, all instances of Atlas will enter this block. This may not be desirable. Please review. repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java Lines 104 (patched) <https://reviews.apache.org/r/66253/#comment280918> Why eat all exceptions here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java Lines 71 (patched) <https://reviews.apache.org/r/66253/#comment280916> migrationInProgress doesn't seem to be used. Please review and remove. repository/src/test/java/org/apache/atlas/repository/migration/RelationshipMappingTest.java Lines 66 (patched) <https://reviews.apache.org/r/66253/#comment280914> It is not clear what this assert looks for? - 'key' is formed by the test in getKey() method - this assert is validating that getKey() returns the right value? repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java Lines 86 (patched) <https://reviews.apache.org/r/66253/#comment280913> 'configuration' doesn't seem to be used in this class. If this is not needed, please remove. webapp/src/main/java/org/apache/atlas/web/service/ServiceState.java Lines 51 (patched) <https://reviews.apache.org/r/66253/#comment280912> MIGRATION => MIGRATING - Madhan Neethiraj On March 31, 2018, 2:18 a.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66253/ > ----------------------------------------------------------- > > (Updated March 31, 2018, 2:18 a.m.) > > > Review request for atlas, Apoorv Naik, Madhan Neethiraj, Ruchi Solani, Sarath > Subramanian, and Vishal Suvagia. > > > Bugs: ATLAS-2460 > https://issues.apache.org/jira/browse/ATLAS-2460 > > > Repository: atlas > > > Description > ------- > > **Background** > This implementation deals with the 'import into' part of the data migration > process. > > It assumes: > - Export from older cluster is done. > - Generated file has been moved to newer cluster. > > **Implementation** > > During _Atlas_ server startup, the configuration parameter is checked, if > that parameter exists, all services except _DataMigrationService_ is started. > Migration is started. Atlas server is available in _MIGRATION_ mode. It > processes REST calls made only to the _AdminResource_. > > Here's are the udpates: > - New configuration parameter has been added: > _atlas.migration.mode.filename=<name of the file to be imported>_ > This configuration parameter is set by Ambari as part of its migration > orchestration. > - _DataMigrationService_: New service that performs async migration as soon > as Atlas server starts up. > - _MigrationProgressService_: Added. Get progress of import. > - _AdminResource.getStatus()_ Now supplies additional status about migration. > - _ServiceState_ Modified to carry additional status _MIGRATION_. This status > is set by looking at the configuration parameter above. > - _Services_ modified for special handling of _DataMigrationService_. > > > **CURL** > Check status using: > ``` > curl -X GET -u admin:admin -H "Content-Type: application/json" -H > "Cache-Control: no-cache" http://localhost:21000/api/atlas/admin/status > ``` > > **Migration Status** > The above URL in migration mode yields JSON like: > ``` > {"Status":"MIGRATION","MigrationStatus":{"operationStatus":"SUCCESS","startTime":1521738357947,"endTime":1521738359272,"currentIndex":48544}} > ``` > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/AtlasConstants.java f5de1df3 > common/src/main/java/org/apache/atlas/service/Services.java 1267dc92 > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java > 31d20855 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java > 6820a93c > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java > a0060200 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/AtlasGraphSONReader.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONTokensTP2.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtility.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeParsers.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeProcessManager.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/MappedElementCache.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessManager.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/ReaderStatusManager.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/RelationshipTypeCache.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/pc/WorkItemBuilder.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/pc/WorkItemConsumer.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/pc/WorkItemManager.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/JsonNodeManagerTest.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessManagerTest.java > PRE-CREATION > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java > 44090097 > intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java > 1ea961d8 > intg/src/main/java/org/apache/atlas/model/impexp/MigrationStatus.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java c63dc24a > pom.xml bfbb9535 > repository/pom.xml b1d6b1f9 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java > 5672d9dc > > repository/src/main/java/org/apache/atlas/repository/impexp/MigrationProgressService.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/impexp/ZipExportFileNames.java > 351b4753 > > repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/migration/RelationshipCacheGenerator.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java > 66762001 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > 5bec16ed > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasTypeDefGraphStoreV1.java > 1a04418a > > repository/src/test/java/org/apache/atlas/repository/migration/MigrationServiceTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/migration/RelationshipMappingTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > 8257faa1 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java > ac35860d > repository/src/test/resources/stocks-2-0.8-extended-tag.json PRE-CREATION > repository/src/test/resources/stocks-2.zip PRE-CREATION > repository/src/test/resources/stocks-2/atlas-export-info.json PRE-CREATION > repository/src/test/resources/stocks-2/atlas-export-order.json PRE-CREATION > repository/src/test/resources/stocks-2/atlas-typesdef.json PRE-CREATION > webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java > 6681a372 > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > 1b3f2c86 > webapp/src/main/java/org/apache/atlas/web/security/AtlasSecurityConfig.java > f1760e7f > webapp/src/main/java/org/apache/atlas/web/service/ServiceState.java > 3fe8d18c > webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java > aab2bb8f > > > Diff: https://reviews.apache.org/r/66253/diff/6/ > > > Testing > ------- > > **Unit tests** > Unit tests for _AtlasGraphSONReader_ added. > > **Functional tests** > Steps to test file-based import: > - Place the exported file say _/root/atlas-data_ > - Add to _Atlas_ Ambari's customer property: > _atlas.migration.mode.filename=/root/atlas-data_ > - Ambari will prompt for a restart. Restart Atlas. > - On the server view the progress in the logs using: _tail -f > /var/log/atlas/application.log_ > - Use the CURL call mentioned above and view the status and the progress of > the import. > > Steps to test directory-based import: > - Place the exported files say _/root/atlas-data_ > - Add to _Atlas_ Ambari's customer property: > _atlas.migration.mode.filename=/root/atlas-data_ > - Ambari will prompt for a restart. Restart Atlas. > - On the server view the progress in the logs using: _tail -f > /var/log/atlas/application.log_ > - Use the CURL call mentioned above and view the status and the progress of > the import. > > > Thanks, > > Ashutosh Mestry > >
