LiJie20190102 commented on code in PR #13574:
URL: https://github.com/apache/gravitino/pull/13574#discussion_r4129607889
##########
core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java:
##########
@@ -65,14 +64,14 @@ public long size() {
@Override
public <E extends Exception> void withCacheLock(EntityCacheKey key,
ThrowingRunnable<E> action)
throws E {
- opLock.withLockAndThrow(key, action);
+ action.run();
Review Comment:
nit: `CaffeineEntityCache.withCacheLock` validates its arguments with
```
Preconditions.checkArgument(key != null, "Key cannot be null");
Preconditions.checkArgument(action != null, "Action cannot be null");
```
but this implementation now calls action.run() directly. A null action here
would surface as a bare NullPointerException instead of the
IllegalArgumentException callers get from the Caffeine implementation. Could we
add the same two checks to both withCacheLock overloads to keep the
implementations consistent?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
