SENTRY-1795: Delta tables should not have holes (Lei(Eddy) Xu, Reviewed by: Alex Kolbasov, Vamsee Yarlagadda)
Change-Id: I61a530b41892350dd0f3e6b7d5907b7220180384 Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23525 Tested-by: Jenkins User Reviewed-by: Lei Xu <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/de52c7e4 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/de52c7e4 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/de52c7e4 Branch: refs/for/cdh5-1.5.1_ha Commit: de52c7e4fbc059b944aa77e82349ea759c0aa24a Parents: d280f2d Author: Vamsee Yarlagadda <[email protected]> Authored: Tue Jun 6 18:38:53 2017 -0700 Committer: Lei Xu <[email protected]> Committed: Tue Jun 6 20:10:18 2017 -0700 ---------------------------------------------------------------------- .../sentry/provider/db/service/model/MSentryPathChange.java | 6 ++---- .../sentry/provider/db/service/model/MSentryPermChange.java | 5 ++--- .../org/apache/sentry/provider/db/service/model/package.jdo | 4 ++-- .../provider/db/service/persistent/DeltaTransactionBlock.java | 6 ++++-- .../sentry/provider/db/service/persistent/TestSentryStore.java | 3 ++- 5 files changed, 12 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/de52c7e4/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java index d11f37f..58878e5 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java @@ -68,14 +68,12 @@ public class MSentryPathChange implements MSentryChange { private long createTimeMs; private long notificationID; - public MSentryPathChange(PathsUpdate pathChange) throws TException { + public MSentryPathChange(long changeID, PathsUpdate pathChange) throws TException { // Each PathsUpdate maps to a MSentryPathChange object. // The PathsUpdate is generated from a HMS notification log, // the notification ID is stored as seqNum and // the notification update is serialized as JSON string. - // - // See SENTRY-1643. changeID is set after increasing 1 of the "max(changeID)" fetched from - // the table, to avoid holes between changeIDs. it is subjected to change. + this.changeID = changeID; this.notificationID = pathChange.getSeqNum(); this.pathChange = pathChange.JSONSerialize(); this.createTimeMs = System.currentTimeMillis(); http://git-wip-us.apache.org/repos/asf/sentry/blob/de52c7e4/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java index 1cb1a1f..e29e780 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java @@ -65,9 +65,8 @@ public class MSentryPermChange implements MSentryChange { private String permChange; private long createTimeMs; - public MSentryPermChange(PermissionsUpdate permChange) throws TException { - // See SENTRY-1643. changeID is set after increasing 1 of the "max(changeID)" fetched from - // the table, to avoid holes between changeIDs. it is subjected to change. + public MSentryPermChange(long changeID, PermissionsUpdate permChange) throws TException { + this.changeID = changeID; this.permChange = permChange.JSONSerialize(); this.createTimeMs = System.currentTimeMillis(); } http://git-wip-us.apache.org/repos/asf/sentry/blob/de52c7e4/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo index 02f5a3b..2c21957 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo @@ -243,7 +243,7 @@ </class> <class name="MSentryPermChange" table="SENTRY_PERM_CHANGE" identity-type="application" detachable="true"> - <field name="changeID" primary-key="true" value-strategy="increment" key-cache-size="1"> + <field name="changeID" primary-key="true"> <column name="CHANGE_ID" jdbc-type="BIGINT" allows-null="false"/> </field> <field name ="permChange"> @@ -255,7 +255,7 @@ </class> <class name="MSentryPathChange" table="SENTRY_PATH_CHANGE" identity-type="application" detachable="true"> - <field name="changeID" primary-key="true" value-strategy="increment" key-cache-size="1"> + <field name="changeID" primary-key="true"> <column name="CHANGE_ID" jdbc-type="BIGINT" allows-null="false"/> </field> <field name="notificationID"> http://git-wip-us.apache.org/repos/asf/sentry/blob/de52c7e4/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java index 1a80300..82a5600 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java @@ -85,9 +85,11 @@ public class DeltaTransactionBlock implements TransactionBlock<Object> { // changeID is trying to be persisted twice, the transaction would // fail. if (update instanceof PermissionsUpdate) { - pm.makePersistent(new MSentryPermChange((PermissionsUpdate) update)); + long lastChangeID = SentryStore.getLastProcessedChangeIDCore(pm, MSentryPermChange.class); + pm.makePersistent(new MSentryPermChange(lastChangeID + 1, (PermissionsUpdate) update)); } else if (update instanceof PathsUpdate) { - pm.makePersistent(new MSentryPathChange((PathsUpdate) update)); + long lastChangeID = SentryStore.getLastProcessedChangeIDCore(pm, MSentryPathChange.class); + pm.makePersistent(new MSentryPathChange(lastChangeID + 1, (PathsUpdate) update)); // Notification id from PATH_UPDATE entry is made persistent in // SENTRY_LAST_NOTIFICATION_ID table. pm.makePersistent(new MSentryHmsNotification(update.getSeqNum())); http://git-wip-us.apache.org/repos/asf/sentry/blob/de52c7e4/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index 7f9a661..201c4fa 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -125,6 +125,7 @@ public class TestSentryStore extends org.junit.Assert { policyFilePath = new File(dataDir, "local_policy_file.ini"); conf.set(ServerConfig.SENTRY_STORE_GROUP_MAPPING_RESOURCE, policyFilePath.getPath()); + conf.setInt(ServerConfig.SENTRY_STORE_TRANSACTION_RETRY, 10); sentryStore = new SentryStore(conf); } @@ -2513,7 +2514,7 @@ public class TestSentryStore extends org.junit.Assert { new PermissionsUpdate(seqNumGenerator.getAndIncrement(), false); tbs.add(new DeltaTransactionBlock(update)); try { - tm.executeTransaction(tbs); + tm.executeTransactionBlocksWithRetry(tbs); } catch (Exception e) { LOGGER.error("Failed to execute permission update transaction", e); fail(String.format("Transaction failed: %s", e.getMessage()));
