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/07/22 12:24:05 UTC

[GitHub] [flink] rkhachatryan opened a new pull request, #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

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

   ## What is the purpose of the change
   
   Please see ticket description (FLINK-28647).
   
   ## Brief changelog
   
   - update the docs (checkpoint folder **is** deleted but with a delay)
   - remove additional error handling (just fail in case of folder deletion failure)
   
   ## Verifying this change
   
   This change is a trivial rework without any test coverage.
   
   ## 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/Mesos, 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? no
   


-- 
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] masteryhx commented on a diff in pull request #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
masteryhx commented on code in PR #20346:
URL: https://github.com/apache/flink/pull/20346#discussion_r932898907


##########
docs/content/docs/ops/state/savepoints.md:
##########
@@ -255,10 +255,8 @@ of the old job will not be deleted by Flink
 
 2. [Native](#savepoint-format) format supports incremental RocksDB savepoints. For those savepoints Flink puts all
 SST files inside the savepoints directory. This means such savepoints are self-contained and relocatable.
-However, when restored in CLAIM mode, subsequent checkpoints might reuse some SST files, which
-in turn might block deleting the savepoints directory at the time the savepoint is subsumed. Later
-on Flink will delete the reused shared SST files, but it won't retry deleting the savepoints directory.
-Therefore, it is possible Flink leaves an empty savepoints directory if it was restored in CLAIM mode.    
+Please note that, when restored in CLAIM mode, subsequent checkpoints might reuse some SST files, which
+might delay the deletion the savepoints directory.

Review Comment:
   IIUC, It works as you modified just after [FLINK-25872](https://issues.apache.org/jira/browse/FLINK-25872), right ?
   Do we have some validations about 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 #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on code in PR #20346:
URL: https://github.com/apache/flink/pull/20346#discussion_r935326212


##########
docs/content/docs/ops/state/savepoints.md:
##########
@@ -255,10 +255,8 @@ of the old job will not be deleted by Flink
 
 2. [Native](#savepoint-format) format supports incremental RocksDB savepoints. For those savepoints Flink puts all
 SST files inside the savepoints directory. This means such savepoints are self-contained and relocatable.
-However, when restored in CLAIM mode, subsequent checkpoints might reuse some SST files, which
-in turn might block deleting the savepoints directory at the time the savepoint is subsumed. Later
-on Flink will delete the reused shared SST files, but it won't retry deleting the savepoints directory.
-Therefore, it is possible Flink leaves an empty savepoints directory if it was restored in CLAIM mode.    
+Please note that, when restored in CLAIM mode, subsequent checkpoints might reuse some SST files, which
+might delay the deletion the savepoints directory.

Review Comment:
   Right. No, there are no validations.



-- 
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 pull request #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on PR #20346:
URL: https://github.com/apache/flink/pull/20346#issuecomment-1192564645

   Thanks for the review @fredia 
   @dawidwys would you like to take a look 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

[GitHub] [flink] fredia commented on pull request #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
fredia commented on PR #20346:
URL: https://github.com/apache/flink/pull/20346#issuecomment-1192540738

   Thanks for the PR, LGTM(non-blinding).


-- 
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 #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #20346:
URL: https://github.com/apache/flink/pull/20346#issuecomment-1192530151

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "1f802932319b3d68cfe4b02593ef714603acf1f5",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1f802932319b3d68cfe4b02593ef714603acf1f5",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 1f802932319b3d68cfe4b02593ef714603acf1f5 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] rkhachatryan merged pull request #20346: [FLINK-28647] Remove separate error handling and adjust documentation for CLAIM mode + RocksDB native savepoint

Posted by GitBox <gi...@apache.org>.
rkhachatryan merged PR #20346:
URL: https://github.com/apache/flink/pull/20346


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