----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70236/#review213861 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/patches/PatchContext.java Lines 41 (patched) <https://reviews.apache.org/r/70236/#comment299965> Patch context instances shouldn't need to be serialized. Please review and remove "implements Serializable" and Json serailizaion annotations. repository/src/main/java/org/apache/atlas/repository/patches/PatchContext.java Lines 79 (patched) <https://reviews.apache.org/r/70236/#comment299972> are equals(), hashCode() and toString() methods necessary for PatchContext? repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatchHandler.java Lines 101 (patched) <https://reviews.apache.org/r/70236/#comment299968> - consider changing the log level to "debug" - to avoid large number of logs for Atlas instances having millions of entities. - consider adding "info" level logs at the start and end of each entity-type - line #76, #124 - consider logging progress periodically, like (in #123): if (entityCount % 1000 == 0) { LOG.info("JavaPatch({}): processed {} entities", getPatchId(), entityCount); } repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatchHandler.java Lines 104 (patched) <https://reviews.apache.org/r/70236/#comment299963> isPatchApplied will be false only if there are no entity that needed unique-key property to be set. Such cases should be treated as success, rather than skipped - since the patch has nothing to do. repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatchHandler.java Lines 120 (patched) <https://reviews.apache.org/r/70236/#comment299971> IndexException could be due to presence of multiple entities with the same unique key value. Instead of treating this as a failure and abort processing, consider adding a warn log and continue: LOG.warn("JavaPatch({}): failed to update entity guid={}, typeName={}, attrName={}, attrValue={}", getPatchId(), typeName, attribute.getName(), uniqAttrValue); Above log will be helpful in manually fixing the condition. repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatchHandler.java Lines 132 (patched) <https://reviews.apache.org/r/70236/#comment299973> Consider moving this method to base class 'AtlasJavaPatchHandler', as this could be useful for other Java patches as well. repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java Lines 200 (patched) <https://reviews.apache.org/r/70236/#comment299974> Every startup will print this info log, followed by a "ignoring" log (UniqueAttributePatchHandler.java#64). Consider the following: for (AtlasJavaPatchHandler patch : patches) { PatchStatus patchStatus = patch.getPatchStatus(); if (patchStatus == APPLIED || patchStatus == SKIPPED) { LOG.info("Ignoring java patch {}; status: {}", patch.getPatchId(), patchStatus); } else { patch.applyPatch(); } } repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasGraphUtilsV2.java Lines 562 (patched) <https://reviews.apache.org/r/70236/#comment299964> Consider renaming this method: findEntityVerticesByType() => findActiveEntityVerticesByType() - madhan On March 20, 2019, 9:29 p.m., Sarath Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70236/ > ----------------------------------------------------------- > > (Updated March 20, 2019, 9:29 p.m.) > > > Review request for atlas, Ashutosh Mestry, Kapildeo Nayak, madhan, Nikhil > Bonte, and Nixon Rodrigues. > > > Bugs: ATLAS-3077 > https://issues.apache.org/jira/browse/ATLAS-3077 > > > Repository: atlas > > > Description > ------- > > We have Atlas typedef patches available to update out of box type > definitions. This JIRA will introduce a new java patch framework to handle > changes to the graph database not specific to type changes. > > For e.g. In case of migration, to add/update property keys in vertices - java > patch framework can be used, which will be run at the atlas startup time > during bootstrap. > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/model/patches/AtlasPatch.java cdf2441d2 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java > 4805435c7 > > repository/src/main/java/org/apache/atlas/repository/patches/AtlasJavaPatchHandler.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/patches/PatchContext.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatchHandler.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java > 337a6db6c > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasGraphUtilsV2.java > 2d5fd9706 > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > 01bdcf7e2 > > > Diff: https://reviews.apache.org/r/70236/diff/2/ > > > Testing > ------- > > Verified with migrated data that patch was applied cleanly > > > Thanks, > > Sarath Subramanian > >
