[GitHub] [flink] xintongsong commented on a change in pull request #13500: [FLINK-19180][state backends] Make RocksDB respect managed memory fraction

[GitBox <gi...@apache.org>]

xintongsong commented on a change in pull request #13500:
URL: https://github.com/apache/flink/pull/13500#discussion_r502362191



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/state/StateBackendLoader.java
##########
@@ -233,6 +236,27 @@ public static StateBackend fromApplicationOrConfigOrDefault(
 		return backend;
 	}
 
+	/**
+	 * Checks whether state backend uses managed memory, without having to deserialize or load the state backend.
+	 * @param config Cluster configuration.
+	 * @param fromApplicationStateBackendName Class name of application-defined state backend, or null if not defined.
+	 * @return Whether the state backend uses managed memory.
+	 */
+	public static boolean stateBackendFraomApplicationOrConfigOrDefaultUseManagedMemory(
+			Configuration config,
+			@Nullable String fromApplicationStateBackendName) {
+
+		checkNotNull(config, "config");
+
+		if (fromApplicationStateBackendName != null) {
+			return Objects.equals(fromApplicationStateBackendName, ROCKSDB_STATE_BACKEND_CLASS_NAME);
+		}
+
+		final String stateBackendFactory = config.getString(CheckpointingOptions.STATE_BACKEND);
+		return Objects.equals(stateBackendFactory, ROCKSDB_STATE_BACKEND_NAME) ||

Review comment:
       Offline discussed with @Myasuka. We decided to do the following.
   - Add an interface `boolean useManagedMemory()` to `StateBackend`, with a default implementation returns `false`, and `RocksDBStateBackend` returns `true`. In this way, custom state backends who uses managed memory can simply override this interface.
   - For backends specified in application, instead of the class name, we record the return value of this interface.
   Makes sense. This avoids extra deserializations of the state backend.
   - For backends specified in cluster configuration, we will have to create the state backend from the factory to get the return value of `useManagedMemory()`. This should be acceptable because `StateBackend` is designed to be created for every operator, thus the cost of creating it should not be heavy.




----------------------------------------------------------------
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:
users@infra.apache.org