[ZEPPELIN-6579] Make notebook tree reload safe for concurrent note operations
### What is this PR for?
`NoteManager` locates a note through two separate pieces of state: `notesInfo` maps a note id to its path, and `root` holds the folder tree that the path is walked against. A lookup uses both in sequence, so the two have to agree.
`reloadNotes()` replaced them one at a time:
```java
public void reloadNotes() throws IOException {
this.root = new Folder("/", notebookRepo, noteCache, zConf); // (1) tree becomes empty
this.trash = this.root.getOrCreateFolder(TRASH_FOLDER);
init(); // (2) new mapping, (3) refill tree
}
```
Neither field is `volatile` and nothing is held while they are swapped, so a concurrent `processNote()` can observe a mapping and a tree that belong to different generations:
| time | reloading thread | note request thread | state |
|---|---|---|---|
| t1 | installs an empty tree | | mapping: old (complete) / tree: **empty** |
| t2 | | `notesInfo.containsKey(noteId)` passes | the id is still in the old mapping |
| t3 | | walks the path in the tree, finds nothing | **throws** |
| t4 | installs the new mapping | | |
| t5 | refills the tree, one note at a time | | notes not inserted yet still fail |
The guard in `processNote()` only checks `notesInfo`, so it passes and the failure surfaces one line later in `getNoteNode()`:
```
java.io.IOException: Can not find note: /E2E_TEST_FOLDER/TestNotebook_...
at org.apache.zeppelin.notebook.NoteManager.getNoteNode
at org.apache.zeppelin.notebook.NoteManager.processNote
at org.apache.zeppelin.rest.NotebookRestApi.updateParagraph
```
`IOException` is not mapped to a specific status, so `WebApplicationExceptionMapper` turns it into **HTTP 500** for a note that was never removed. Everything that goes through `processNote()` is affected: reading a note, updating a paragraph, creating, deleting and moving notes, and listing the notebook.
This PR holds the tree, the trash folder and the mapping in one immutable `NoteTree` and publishes it with a single `volatile` write. `buildNoteTree()` fills the new tree locally and returns it; only then is it assigned. The tree-walking helpers (`getNoteNode`, `getFolder`, `getOrCreateFolder`, `isNotePathAvailable`) take the tree as a parameter, and callers that need both pieces of state read the reference once, so a lookup resolves the mapping and the tree against the same generation. Those helpers are `static` so that the compiler prevents them from reaching back to the field.
### Scope and related issues
**#5325** (`[ZEPPELIN-5858]`) is open against the same class and restructures `removeNote`, `moveNote` and `moveFolder` with `synchronized (this)`. It targets a different race (two mutators duplicating a note) and its monitor does not cover `reloadNotes()`, so neither change subsumes the other. Whichever merges second will need a rebase.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Build the new tree, trash folder and mapping in `buildNoteTree()` before publishing them
* [x] - Hold the three in an immutable `NoteTree` published through a single `volatile` write
* [x] - Pass the tree into the tree-walking helpers so one lookup uses one generation
* [x] - Add a regression test that reloads while other threads read notes
* [x] - Confirm the test fails without the fix and passes with it
### What is the Jira issue?
* [ZEPPELIN-6579](https://issues.apache.org/jira/browse/ZEPPELIN-6579)
### How should this be tested?
New test `NoteManagerTest#testConcurrentReloadAndProcessNote`: it saves 50 notes, then runs `reloadNotes()` in a loop on one thread while four threads keep calling `processNote()` for every note, and asserts that no lookup fails or returns nothing.
```bash
export JAVA_HOME=$(/usr/libexec/java_home -v 11)
./mvnw package -pl zeppelin-server --am -Dtest=NoteManagerTest -DfailIfNoTests=false
```
Result with the fix: `Tests run: 7, Failures: 0, Errors: 0`.
Reverting only the production change makes the new test fail on every reader thread with `java.io.IOException: Can not find note: /prod/note_0` thrown from `NoteManager.getNoteNode` via `NoteManager.processNote`, which is the stack from the ticket; with the fix it passes.
Also run, to cover the callers of the reload path:
```bash
./mvnw package -pl zeppelin-server --am \
-Dtest='NotebookTest#testReloadAllNotes+testReloadAndSetInterpreter' -DfailIfNoTests=false
```
Result: `Tests run: 2, Failures: 0, Errors: 0`.
Not verified locally: the full `NotebookTest` and `NotebookServerTest` classes, which start real remote interpreter processes and time out in my environment, and `NotebookRepoSyncTest`. Those are left to CI.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5357 from big-cir/ZEPPELIN-6579.
Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>Documentation: User Guide
Mailing Lists: User and Dev mailing list
Continuous Integration:
Contributing: Contribution Guide
Issue Tracker: Jira
License: Apache 2.0
Zeppelin, a web-based notebook that enables interactive data analytics. You can make beautiful data-driven, interactive and collaborative documents with SQL, Scala and more.
Core features:
To know more about Zeppelin, visit our web site https://zeppelin.apache.org
Please go to install to install Apache Zeppelin from binary package.
Please check Build from source to build Zeppelin from source.