vkulichenko commented on a change in pull request #8820:
URL: https://github.com/apache/ignite/pull/8820#discussion_r586875939
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheSemaphoreImpl.java
##########
@@ -662,6 +665,28 @@ private void initializeSemaphore() throws
IgniteCheckedException {
}
}
+ /** {@inheritDoc} */
+ @Override public <T> IgniteFuture<T>
acquireAndExecute(IgniteCallable<IgniteFuture<T>> callable,
+ int numPermits)
throws Exception {
+ acquire(numPermits);
+
+ IgniteFuture<T> future = null;
+ try {
+ future = callable.call();
+ } finally {
+ release(numPermits);
Review comment:
Is this release required? Since `call()` here is asynchronous, permits
will be released before completion, which seems wrong. Also, there is another
`release` invocation, which happens within the future listener - that one looks
correct.
##########
File path: modules/core/src/main/java/org/apache/ignite/IgniteSemaphore.java
##########
@@ -244,6 +247,16 @@ public boolean tryAcquire(int permits, long timeout,
TimeUnit unit)
*/
public void acquire(int permits) throws IgniteInterruptedException;
+ /**
+ * Acquires the given semaphore, executes the given callable and schedules
the release of permits asynchronously
+ *
+ * @param callable the callable to execute
+ * @param numPermits the number of permits to acquire
+ * @throws Exception if the callable throws an exception
+ */
+ public <T> IgniteFuture<T>
acquireAndExecute(IgniteCallable<IgniteFuture<T>> callable,
+ int numPermits) throws
Exception;
+
Review comment:
I'm not sure I understand why callable returns `IgniteFuture<T>` rather
than just `T`. I saw a comment in the ticket, but it doesn't really provide
justification. It probably makes sense to have a separate thread on the dev
list regarding this - we should have a discussion if there are different
opinions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
