You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/11/28 02:33:01 UTC
[GitHub] [flink-table-store] JingsongLi opened a new pull request, #405: [FLINK-30223] Refactor Lock to provide Lock.Factory
JingsongLi opened a new pull request, #405:
URL: https://github.com/apache/flink-table-store/pull/405
For the core, it should not see too many Flink Table concepts, such as database and tableName. It only needs to create a Lock.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] tsreaper merged pull request #405: [FLINK-30223] Refactor Lock to provide Lock.Factory
Posted by GitBox <gi...@apache.org>.
tsreaper merged PR #405:
URL: https://github.com/apache/flink-table-store/pull/405
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #405: [FLINK-30223] Refactor Lock to provide Lock.Factory
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #405:
URL: https://github.com/apache/flink-table-store/pull/405#discussion_r1033178800
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/operation/Lock.java:
##########
@@ -31,13 +32,57 @@ public interface Lock extends AutoCloseable {
/** Run with lock. */
<T> T runWithLock(Callable<T> callable) throws Exception;
- @Nullable
- static Lock fromCatalog(CatalogLock.Factory lockFactory, ObjectPath tablePath) {
- if (lockFactory == null) {
- return null;
+ /** A factory to create {@link Lock}. */
+ interface Factory extends Serializable {
+ Lock create();
+ }
+
+ static Factory factory(@Nullable CatalogLock.Factory lockFactory, ObjectPath tablePath) {
+ return lockFactory == null
+ ? new EmptyFactory()
+ : new CatalogLockFactory(lockFactory, tablePath);
+ }
+
+ static Factory emptyFactory() {
+ return new EmptyFactory();
+ }
+
+ /** A {@link Factory} creating lock from catalog. */
+ class CatalogLockFactory implements Factory {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CatalogLock.Factory lockFactory;
+ private final ObjectPath tablePath;
+
+ public CatalogLockFactory(CatalogLock.Factory lockFactory, ObjectPath tablePath) {
+ this.lockFactory = lockFactory;
+ this.tablePath = tablePath;
+ }
+
+ @Override
+ public Lock create() {
+ return fromCatalog(lockFactory.create(), tablePath);
}
+ }
- return fromCatalog(lockFactory.create(), tablePath);
+ /** A {@link Factory} creating empty lock. */
+ class EmptyFactory implements Factory {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Lock create() {
+ return new Lock() {
+ @Override
+ public <T> T runWithLock(Callable<T> callable) throws Exception {
+ return callable.call();
+ }
+
+ @Override
+ public void close() {}
+ };
Review Comment:
I think we can return a object to avoid null checking.
I will modify others to remove null checking.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi commented on pull request #405: [FLINK-30223] Refactor Lock to provide Lock.Factory
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on PR #405:
URL: https://github.com/apache/flink-table-store/pull/405#issuecomment-1328450069
This is second refactor PR for https://github.com/apache/flink-table-store/pull/394
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] tsreaper commented on a diff in pull request #405: [FLINK-30223] Refactor Lock to provide Lock.Factory
Posted by GitBox <gi...@apache.org>.
tsreaper commented on code in PR #405:
URL: https://github.com/apache/flink-table-store/pull/405#discussion_r1033077832
##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/operation/Lock.java:
##########
@@ -31,13 +32,57 @@ public interface Lock extends AutoCloseable {
/** Run with lock. */
<T> T runWithLock(Callable<T> callable) throws Exception;
- @Nullable
- static Lock fromCatalog(CatalogLock.Factory lockFactory, ObjectPath tablePath) {
- if (lockFactory == null) {
- return null;
+ /** A factory to create {@link Lock}. */
+ interface Factory extends Serializable {
+ Lock create();
+ }
+
+ static Factory factory(@Nullable CatalogLock.Factory lockFactory, ObjectPath tablePath) {
+ return lockFactory == null
+ ? new EmptyFactory()
+ : new CatalogLockFactory(lockFactory, tablePath);
+ }
+
+ static Factory emptyFactory() {
+ return new EmptyFactory();
+ }
+
+ /** A {@link Factory} creating lock from catalog. */
+ class CatalogLockFactory implements Factory {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CatalogLock.Factory lockFactory;
+ private final ObjectPath tablePath;
+
+ public CatalogLockFactory(CatalogLock.Factory lockFactory, ObjectPath tablePath) {
+ this.lockFactory = lockFactory;
+ this.tablePath = tablePath;
+ }
+
+ @Override
+ public Lock create() {
+ return fromCatalog(lockFactory.create(), tablePath);
}
+ }
- return fromCatalog(lockFactory.create(), tablePath);
+ /** A {@link Factory} creating empty lock. */
+ class EmptyFactory implements Factory {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Lock create() {
+ return new Lock() {
+ @Override
+ public <T> T runWithLock(Callable<T> callable) throws Exception {
+ return callable.call();
+ }
+
+ @Override
+ public void close() {}
+ };
Review Comment:
Just return `null`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org