zeppelin icon indicating copy to clipboard operation
zeppelin copied to clipboard

[ZEPPELIN-6269] Handle remove failures in NotebookRepoSync to prevent data inconsistency

Open hyunw9 opened this issue 4 months ago • 2 comments

What is this PR for?

If an IOException occurs while trying to remove a note from one of the repositories, the for loop would terminate immediately. This leaves the note in an inconsistent state where it is deleted from some repositories but not from others, leading to data inconsistency.

Approach

  1. The method will now attempt to remove the note from all configured repositories, even if an error occurs with one of them.
  2. Any repository that fails to remove the note is added to a failedRepos list.
  3. After iterating through all repositories, the method checks the failedRepos list. If the list is not empty, it throws a single IOException that includes information about all the repositories where the removal failed.

What type of PR is it?

Refactoring

Todos

  • [x] - Refactor remove method
  • [x] - Add test for modified method

What is the Jira issue?

How should this be tested?

  • Added simple integration test in NotebookRepoSyncTest.

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? - No
  • Is there breaking changes for older versions? - No
  • Does this needs documentation? - No

hyunw9 avatar Aug 07 '25 03:08 hyunw9

@jongyoul It would be a nicer approach. Let me show you a draft. :)

hyunw9 avatar Aug 10 '25 09:08 hyunw9

@jongyoul I just pushed modified version. How about this approach? if it fails, we can raise exception immediately, so we can avoid out-of-sync status.

hyunw9 avatar Aug 11 '25 12:08 hyunw9