Repository: nifi Updated Branches: refs/heads/master 37488d278 -> 18133988a
NIFI-1984: Ensure that if an Exception is thrown by the 'Deletion Task' when calling NaiveRevisionManager.deleteRevision() that the locking is appropriately cleaned up This closes #510 Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/18133988 Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/18133988 Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/18133988 Branch: refs/heads/master Commit: 18133988a02d80180ee55b91bc5b80807ec9f3de Parents: 37488d2 Author: Mark Payne <[email protected]> Authored: Wed Jun 8 08:57:37 2016 -0400 Committer: Matt Burgess <[email protected]> Committed: Wed Jun 8 10:31:36 2016 -0400 ---------------------------------------------------------------------- .../nifi/web/revision/NaiveRevisionManager.java | 16 +++- .../web/revision/TestNaiveRevisionManager.java | 96 ++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/18133988/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java index c00661d..7bcf9f4 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java @@ -209,7 +209,21 @@ public class NaiveRevisionManager implements RevisionManager { if (successCount == revisionList.size()) { logger.debug("Successfully verified Revision Claim for all revisions {}", claim); - final T taskValue = task.performTask(); + final T taskValue; + try { + taskValue = task.performTask(); + } catch (final Exception e) { + logger.debug("Failed to perform Claim Deletion task. Will relinquish the Revision Claims for the following revisions: {}", revisionList); + + for (final Revision revision : revisionList) { + final RevisionLock revisionLock = getRevisionLock(revision); + revisionLock.unlock(revision, revision, user.getUserName()); + logger.debug("Relinquished lock for {}", revision); + } + + throw e; + } + for (final Revision revision : revisionList) { deleteRevisionLock(revision); logger.debug("Deleted Revision {}", revision); http://git-wip-us.apache.org/repos/asf/nifi/blob/18133988/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java index 303b450..e3148d3 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/test/java/org/apache/nifi/web/revision/TestNaiveRevisionManager.java @@ -615,4 +615,100 @@ public class TestNaiveRevisionManager { assertTrue(component1Seen); assertTrue(component2Seen); } + + @Test(timeout = 5000) + public void testWriteLockReleasedWhenClaimCanceledByRevision() { + final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.SECONDS); + final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1); + + final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim); + assertEquals(1, claim.getRevisions().size()); + assertEquals(component1V1, claim.getRevisions().iterator().next()); + + assertTrue(revisionManager.cancelClaim(component1V1)); + + final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim2); + assertEquals(1, claim2.getRevisions().size()); + assertEquals(component1V1, claim2.getRevisions().iterator().next()); + } + + @Test(timeout = 5000) + public void testWriteLockReleasedWhenClaimCanceledByComponentId() { + final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.SECONDS); + final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1); + + final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim); + assertEquals(1, claim.getRevisions().size()); + assertEquals(component1V1, claim.getRevisions().iterator().next()); + + assertTrue(revisionManager.cancelClaim(COMPONENT_1)); + + final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim2); + assertEquals(1, claim2.getRevisions().size()); + assertEquals(component1V1, claim2.getRevisions().iterator().next()); + } + + @Test(timeout = 5000) + public void testDeleteRevisionUnlocksClaimIfExceptionThrown() { + final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.MINUTES); + final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1); + + final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim); + assertEquals(1, claim.getRevisions().size()); + assertEquals(component1V1, claim.getRevisions().iterator().next()); + + final RuntimeException re = new RuntimeException("Intentional Unit Test Exception"); + try { + revisionManager.deleteRevision(claim, USER_1, () -> { + throw re; + }); + + Assert.fail("deleteRevision() method did not propagate Exception thrown"); + } catch (final RuntimeException e) { + assertTrue(re == e); + } + + // Ensure that we can obtain a read lock + revisionManager.get(COMPONENT_1, rev -> rev); + + final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim2); + assertEquals(1, claim2.getRevisions().size()); + assertEquals(component1V1, claim2.getRevisions().iterator().next()); + } + + @Test(timeout = 5000) + public void testUpdateRevisionUnlocksClaimIfExceptionThrown() { + final RevisionManager revisionManager = new NaiveRevisionManager(2, TimeUnit.MINUTES); + final Revision component1V1 = new Revision(1L, CLIENT_1, COMPONENT_1); + + final RevisionClaim claim = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim); + assertEquals(1, claim.getRevisions().size()); + assertEquals(component1V1, claim.getRevisions().iterator().next()); + + final RuntimeException re = new RuntimeException("Intentional Unit Test Exception"); + try { + revisionManager.updateRevision(claim, USER_1, () -> { + throw re; + }); + + Assert.fail("updateRevision() method did not propagate Exception thrown"); + } catch (final RuntimeException e) { + assertTrue(re == e); + } + + // Ensure that we can obtain a read lock + revisionManager.get(COMPONENT_1, rev -> rev); + + final RevisionClaim claim2 = revisionManager.requestClaim(component1V1, USER_1); + assertNotNull(claim2); + assertEquals(1, claim2.getRevisions().size()); + assertEquals(component1V1, claim2.getRevisions().iterator().next()); + } } \ No newline at end of file
