You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/08/31 01:58:11 UTC
[GitHub] [hudi] nsivabalan opened a new pull request, #6549: [HUDI-4751] Adding tests to InProcessLockProvider to test unlock by diff thread
nsivabalan opened a new pull request, #6549:
URL: https://github.com/apache/hudi/pull/6549
### Change Logs
There are some callers of txnManager apis which does not set the current owner instant. Lets try to set all owners since that would ensure a diff owner may not unlock by mistake.
### Impact
No detected bugs yet. just as precautionary measure.
**Risk level: low **
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on code in PR #6549:
URL: https://github.com/apache/hudi/pull/6549#discussion_r959890634
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
Review Comment:
sure, will fix it. my bad.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a diff in pull request #6549: [HUDI-4751] Add tests to InProcessLockProvider to test unlock by diff thread
Posted by GitBox <gi...@apache.org>.
xushiyan commented on code in PR #6549:
URL: https://github.com/apache/hudi/pull/6549#discussion_r959882710
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
Review Comment:
this is not used?
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
+ final AtomicBoolean writer3Completed = new AtomicBoolean(false);
+
+ // Main test thread
+ Assertions.assertTrue(inProcessLockProvider.tryLock());
+
+ // Another writer thread
+ Thread writer2 = new Thread(() -> {
Review Comment:
why need writer2 when you have writer3 below?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1234587641
Fixed the title appropriately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1234802309
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
}, {
"hash" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11096",
"triggerID" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 19c19f5b2acdc635c3235deeb07a89df59da9f9f Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11096)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on code in PR #6549:
URL: https://github.com/apache/hudi/pull/6549#discussion_r960935122
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
+ final AtomicBoolean writer3Completed = new AtomicBoolean(false);
+
+ // Main test thread
+ Assertions.assertTrue(inProcessLockProvider.tryLock());
+
+ // Another writer thread
+ Thread writer2 = new Thread(() -> {
Review Comment:
here is the context
writer2 unlocks w/o actually locking. if a non owner unlocks, we don't throw any exception as such. but intrinsically, the lock is still acquired by the original owner (wirter1)
and so, we again try to acquire the lock by writer3. if at all the unlocking by writer2 succeeded, writer3 should be able to acquire the lock. but we expect it to fail since writer1 is the orignal owner of the lock.
and hence we need 3 writers to verify this scenario.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #6549: [HUDI-4751] Add tests to InProcessLockProvider to test unlock by diff thread
Posted by GitBox <gi...@apache.org>.
xushiyan commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1233268544
PR title should also indicate there are non-test changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan merged pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan merged PR #6549:
URL: https://github.com/apache/hudi/pull/6549
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Adding tests to InProcessLockProvider to test unlock by diff thread
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1232369721
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Adding tests to InProcessLockProvider to test unlock by diff thread
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1232607757
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1234593349
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
}, {
"hash" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066)
* 19c19f5b2acdc635c3235deeb07a89df59da9f9f UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on code in PR #6549:
URL: https://github.com/apache/hudi/pull/6549#discussion_r959890858
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
+ final AtomicBoolean writer3Completed = new AtomicBoolean(false);
+
+ // Main test thread
+ Assertions.assertTrue(inProcessLockProvider.tryLock());
+
+ // Another writer thread
+ Thread writer2 = new Thread(() -> {
Review Comment:
sorry, some refactoring residues. will clean it up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Adding tests to InProcessLockProvider to test unlock by diff thread
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1232373116
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on code in PR #6549:
URL: https://github.com/apache/hudi/pull/6549#discussion_r960935122
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestInProcessLockProvider.java:
##########
@@ -160,6 +160,48 @@ public void testTryLockReAcquisitionByDifferentThread() {
});
}
+ @Test
+ public void testTryUnLockByDifferentThread() {
+ InProcessLockProvider inProcessLockProvider = new InProcessLockProvider(lockConfiguration, hadoopConfiguration);
+ final AtomicBoolean writer2Completed = new AtomicBoolean(false);
+ final AtomicBoolean writer3Completed = new AtomicBoolean(false);
+
+ // Main test thread
+ Assertions.assertTrue(inProcessLockProvider.tryLock());
+
+ // Another writer thread
+ Thread writer2 = new Thread(() -> {
Review Comment:
here is the context
writer2 unlocks w/o actually locking. if a non owner unlocks, we don't throw any exception as such. but intrinsically, the lock is still acquired by the original owner (wirter1)
and so, we again try to acquire the lock by writer3. if at all the unlocking by writer2 succeeded above, writer3 should be able to acquire the lock. but we expect it to fail since writer1 is the orignal owner of the lock.
and hence we need 3 writers to verify this scenario.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #6549: [HUDI-4751] Fix owner instants for transaction manager api callers
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #6549:
URL: https://github.com/apache/hudi/pull/6549#issuecomment-1234600957
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066",
"triggerID" : "4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8",
"triggerType" : "PUSH"
}, {
"hash" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11096",
"triggerID" : "19c19f5b2acdc635c3235deeb07a89df59da9f9f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fc4a7273ab568e11d65e9d5e0a9dfc458371fb8 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11066)
* 19c19f5b2acdc635c3235deeb07a89df59da9f9f Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=11096)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org