MartijnVisser commented on code in PR #29307:
URL: https://github.com/apache/flink/pull/29307#discussion_r4122252940


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/factories/TestValuesTableFactory.java:
##########
@@ -194,12 +195,21 @@ public final class TestValuesTableFactory
     // 
--------------------------------------------------------------------------------------------
 
     private static final AtomicInteger idCounter = new AtomicInteger(0);
-    private static final Map<String, Collection<Row>> registeredData = new 
HashMap<>();
-    private static final Map<String, Collection<RowData>> registeredRowData = 
new HashMap<>();
+    private static final Map<String, Collection<Row>> registeredData = new 
ConcurrentHashMap<>();

Review Comment:
   `TestValuesModelFactory.REGISTERED_DATA` needs the same: it's still a plain 
`HashMap` written by parallel `ML_PREDICT` invocations. If you want to keep the 
concurrency, I think it needs its own ticket.



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/testutils/CommonSemanticTestBase.java:
##########
@@ -58,9 +60,21 @@
  * whether the execution result is semantically correct.
  */
 @ExtendWith(MiniClusterExtension.class)
+@Execution(ExecutionMode.CONCURRENT)

Review Comment:
   On master these never run concurrently: the root pom defaults classes and 
methods to `same_thread`, and surefire forks are separate JVMs. So concurrency 
can't be what breaks FLINK-40670. This annotation is what adds it.



-- 
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]

Reply via email to