You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by kl0u <gi...@git.apache.org> on 2017/10/25 16:52:42 UTC
[GitHub] flink pull request #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packag...
GitHub user kl0u opened a pull request:
https://github.com/apache/flink/pull/4906
[FLINK-7908][FLINK-7824][QS] Restructure QS packages and put QS jars in opt/.
## What is the purpose of the change
Make the Queryable State more usable by:
1) Reducing the dependencies the `client` jar (used by the user) transitively brings.
2) Putting the core and client jars in the `opt/` folder. From there, to activate QS the user has to put the core jar in the lib/ folder before starting the cluster, and use the client jar as a dependency to his/her program.
## Brief change log
Creating the `core` and `client` modules in the queryable state dir and moving classes around.
In addition, now the `client` module becomes a dependency of the `flink-runtime`.
Places the jars corresponding the aforementioned modules to the `opt` dir.
## Verifying this change
This change is a simple restructuring of the queryable state module, no classes are introduced or deleted.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): yes, adds the `flink-queryable-state-client` dependency to `flink-runtime`.
## Documentation
- Does this pull request introduce a new feature? NO, but it needs documentation, which is PENDING.
R @aljoscha
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kl0u/flink qs-restructure
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4906.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4906
----
commit 057f14d833e56fa731262384a636af13f08623d5
Author: kkloudas <kk...@gmail.com>
Date: 2017-10-24T10:16:08Z
[FLINK-7908][QS] Restructure the queryable state module.
The QS module is split into core and client. The core should
be put in the lib folder to enable queryable state, while the
client is the one that the user will program against. The
reason for the restructuring in mainly to remove the dependency
on the flink-runtime from the user's program.
commit dc9d5964bcdd3269fe01e8fe2ab7be74d90dd22c
Author: kkloudas <kk...@gmail.com>
Date: 2017-10-24T14:12:27Z
[FLINK-7824][QS] Put the QS modules in the opt folder.
Now the user can find the jars in the opt/ folder and
he can activate QS by putting the core jar in the lib/
folder and program against the client jar.
----
---
[GitHub] flink pull request #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packag...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/4906#discussion_r147106176
--- Diff: flink-queryable-state/flink-queryable-state-client-java/src/main/java/org/apache/flink/queryablestate/client/VoidNamespace.java ---
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.queryablestate.client;
+
+import java.io.ObjectStreamException;
+
+/**
+ * Singleton placeholder class for state without a namespace.
+ *
+ * <p><b>THIS WAS COPIED FROM RUNTIME SO THAT WE AVOID THE DEPENDENCY.</b>
+ */
+public final class VoidNamespace {
--- End diff --
Can these be package private?
---
[GitHub] flink issue #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packages and ...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4906
Sorry for joining the party late, but I don't think that it is a good idea to duplicate code by copying code from one module into the other, as happened for the `FutureUtils` for example. The proper solution would be to move the `FutureUtils` into `flink-core` such that they are accessible from all places. Doing differently will lead us to maintenance hell quite quickly.
---
[GitHub] flink pull request #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packag...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/4906#discussion_r147106214
--- Diff: flink-queryable-state/flink-queryable-state-client-java/src/main/java/org/apache/flink/queryablestate/FutureUtils.java ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.queryablestate;
+
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * Javadoc.
--- End diff --
Missing Javadoc.
---
[GitHub] flink issue #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packages and ...
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the issue:
https://github.com/apache/flink/pull/4906
Thanks a lot for the review @aljoscha ! Waiting for travis and then merging.
---
[GitHub] flink issue #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packages and ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/4906
Btw, I'm assuming you also tried this with a cluster where you put the "core" jar in lib and then could query the state of a running job?
---
[GitHub] flink pull request #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packag...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4906
---
[GitHub] flink issue #4906: [FLINK-7908][FLINK-7824][QS] Restructure QS packages and ...
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/4906
LGTM with the name changes!
Excellent work. 👍
---