You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "XComp (via GitHub)" <gi...@apache.org> on 2023/04/11 07:22:49 UTC

[GitHub] [flink] XComp opened a new pull request, #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

XComp opened a new pull request, #22377:
URL: https://github.com/apache/flink/pull/22377

   ## What is the purpose of the change
   
   FLINK-31593 revealed some non-determinism in the checkpoint generation for migration tests. The actual issue is covered in FLINK-31766. As a workaround, we're disabling changelog backend for now.
   
   ## Brief change log
   
   * Went through any still existing tests based on 369088f0 and disabled changelog backend for RocksDB backend-based tests only.
   
   ## Verifying this change
   
   Test data can be generated and verified for those tests.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: yes
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable


-- 
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] flinkbot commented on pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22377:
URL: https://github.com/apache/flink/pull/22377#issuecomment-1502819839

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "acc37e52cec0a6f45450b303e232694cd246d821",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "acc37e52cec0a6f45450b303e232694cd246d821",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * acc37e52cec0a6f45450b303e232694cd246d821 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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] XComp commented on a diff in pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #22377:
URL: https://github.com/apache/flink/pull/22377#discussion_r1163828497


##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/StatefulJobSnapshotMigrationITCase.java:
##########
@@ -136,6 +136,12 @@ public void testSavepoint() throws Exception {
         switch (snapshotSpec.getStateBackendType()) {
             case StateBackendLoader.ROCKSDB_STATE_BACKEND_NAME:
                 env.setStateBackend(new EmbeddedRocksDBStateBackend());
+
+                if (executionMode == ExecutionMode.CREATE_SNAPSHOT) {

Review Comment:
   @rkhachatryan That means that we don't have to create backports but only merge this PR (because for 1.17 I have to disable the changelog backend manually based on `release-1.17.0` and for 1.16 we have the migration test data already generated?!



-- 
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] rkhachatryan commented on a diff in pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "rkhachatryan (via GitHub)" <gi...@apache.org>.
rkhachatryan commented on code in PR #22377:
URL: https://github.com/apache/flink/pull/22377#discussion_r1163316537


##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/StatefulJobSnapshotMigrationITCase.java:
##########
@@ -136,6 +136,9 @@ public void testSavepoint() throws Exception {
         switch (snapshotSpec.getStateBackendType()) {
             case StateBackendLoader.ROCKSDB_STATE_BACKEND_NAME:
                 env.setStateBackend(new EmbeddedRocksDBStateBackend());
+                // disable changelog backend for now to ensure determinism in test data generation
+                // (see FLINK-31766)
+                env.enableChangelogStateBackend(false);

Review Comment:
   nit: this change duplicates line 155, one can be removed
   
   nit: can be scoped to `executionMode == ExecutionMode.CREATE_SNAPSHOT` only



-- 
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] XComp merged pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp merged PR #22377:
URL: https://github.com/apache/flink/pull/22377


-- 
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] XComp commented on a diff in pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on code in PR #22377:
URL: https://github.com/apache/flink/pull/22377#discussion_r1163824425


##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/StatefulJobSnapshotMigrationITCase.java:
##########
@@ -136,6 +136,9 @@ public void testSavepoint() throws Exception {
         switch (snapshotSpec.getStateBackendType()) {
             case StateBackendLoader.ROCKSDB_STATE_BACKEND_NAME:
                 env.setStateBackend(new EmbeddedRocksDBStateBackend());
+                // disable changelog backend for now to ensure determinism in test data generation
+                // (see FLINK-31766)
+                env.enableChangelogStateBackend(false);

Review Comment:
   good catch. I'm gonna update 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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] rkhachatryan commented on a diff in pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "rkhachatryan (via GitHub)" <gi...@apache.org>.
rkhachatryan commented on code in PR #22377:
URL: https://github.com/apache/flink/pull/22377#discussion_r1164249518


##########
flink-tests/src/test/java/org/apache/flink/test/checkpointing/StatefulJobSnapshotMigrationITCase.java:
##########
@@ -136,6 +136,12 @@ public void testSavepoint() throws Exception {
         switch (snapshotSpec.getStateBackendType()) {
             case StateBackendLoader.ROCKSDB_STATE_BACKEND_NAME:
                 env.setStateBackend(new EmbeddedRocksDBStateBackend());
+
+                if (executionMode == ExecutionMode.CREATE_SNAPSHOT) {

Review Comment:
   @XComp I think you're right, backporting isn't necessary.



-- 
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] XComp commented on pull request #22377: [FLINK-31765][runtime][test] Disable changelog backend for RocksDB migration tests

Posted by "XComp (via GitHub)" <gi...@apache.org>.
XComp commented on PR #22377:
URL: https://github.com/apache/flink/pull/22377#issuecomment-1502832658

   @fredia may you have a look at it as well?


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