Skip to content

[ZEPPELIN-6704] Scope personalized streaming output to its execution user - #5479

Open
big-cir wants to merge 3 commits into
apache:masterfrom
big-cir:ZEPPELIN-6704
Open

big-cir wants to merge 3 commits into
apache:masterfrom
big-cir:ZEPPELIN-6704

Conversation

@big-cir

@big-cir big-cir commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

In a personalized note, paragraph results are stored per user, but the incremental output produced while a paragraph runs is not. The interpreter knows who started the execution, yet the output events it sends back carry no owner, so NotebookServer cannot tell who the output belongs to. Personalized incremental output is therefore dropped, and a user sees nothing until the paragraph finishes.

The owner is lost in two places. OutputAppendEvent and OutputUpdateEvent have no user field, and createInterpreterOutput captures only the note and paragraph ids from the interpreter context, even though the authentication info is available there too. Recovering it on the server is not an option either. The shared paragraph's user field is updated only when the note owner runs it, so it can hold a stale user from an earlier run rather than the owner of the current output.

This PR carries the execution owner end to end, in three commits:

  1. Add an optional user field to the interpreter output events and thread it from the interpreter to the server. The interpreter captures the owner when the output is created, before any chunks are produced, so every event from that execution carries the same owner. AppendOutputRunner now keys its buffer by owner as well, because two users running the same paragraph would otherwise have their chunks merged into the same buffer, leaving one chunk with no single owner. Behaviour is unchanged in this commit.
  2. Deliver personalized append and update events to the execution owner through multicastToUser, write updates to that user's own paragraph copy rather than the shared one, and make clear events discard only the owner's output. An interpreter that predates the field reports no owner, and that output is still dropped rather than broadcast.
  3. Add two-user e2e coverage.

Shared mode is untouched. A shared paragraph has only one execution producing output for a given index at a time, so adding the owner to the buffer key does not change its batching behavior. AppendOutputRunnerTest.appendsFromOneExecutionAreStillBatchedIntoOneChunk verifies this.

Two notes on scope. The paragraph output callbacks on RemoteInterpreterProcessListener are renamed to onParagraphOutputAppend, onParagraphOutputUpdated and onParagraphOutputClear: once the owner is added, they have the same erasure as the app output overloads that NotebookServer also implements, so the class would not compile. The Helium app output events carry no owner either, but I could not find any personalized code path that uses them, so they are left out of this PR.

What type of PR is it?

Bug Fix

Todos

  • - Carry the execution owner from the interpreter to NotebookServer
  • - Key the append buffer by owner so concurrent runs are not merged
  • - Route personalized append, update and clear to the execution owner
  • - Add server tests and a two-user e2e test

What is the Jira issue?

How should this be tested?

Server side:

./mvnw package -pl zeppelin-server --am \
  -Dtest="NotebookServerStreamingScopeTest,AppendOutputRunnerTest,RemoteInterpreterEventServerTest" \
  -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false

27 tests pass. NotebookServerStreamingScopeTest covers the routing, including a test that reproduces the original failure mode: the shared paragraph is given a stale owner and a different user runs the paragraph, and the output must still reach the user who started that execution.

The e2e test signs in as two shiro accounts in separate browser contexts, opens one personalized note, and records the PARAGRAPH_APPEND_OUTPUT and PARAGRAPH_UPDATE_OUTPUT frames each socket receives. The test asserts on WebSocket frames rather than rendered UI because the bug is in server-side connection routing. It runs on the authenticated leg:

cd zeppelin-web-angular && npx playwright test e2e/tests/notebook/personalized --project=chromium

Reverting the routing change makes the new tests fail:

NotebookServerStreamingScopeTest.personalizedOutputReachesOnlyItsExecutionOwner
  Wanted but not invoked: connectionManager.multicastToUser("owner", ...)
NotebookServerStreamingScopeTest.personalizedUpdateWritesToTheOwnersCopyNotTheSharedParagraph
  the owner's copy did not receive its own output ==> expected: <1> but was: <0>
NotebookServerStreamingScopeTest.personalizedClearOnlyDiscardsTheOwnersOutput
  the owner's output survived its own clear ==> expected: <null> but was: <%text owned>
personalized-streaming-scope.spec.ts
  no marker in 0 streaming frames
  Expected substring: "RUNNER-ONLY" / Received string: ""

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No. The thrift field is optional, so an older interpreter process simply reports no owner and its personalized output is dropped instead of broadcast.
  • Does this needs documentation? No

…events

Plumbing only: the owner now reaches NotebookServer but routing is unchanged.
Delivery is switched over in the following commit.

@tbonelee tbonelee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A naming suggestion.

user does not say that this value is the owner of one execution. Two kinds of it meet on a single line in onParagraphOutputUpdated:

note.getParagraph(paragraphId).getUserParagraphMap().get(user)

The defect this PR fixes also came from yet another user, the shared paragraph's own field.

I pushed a version renamed to executionOwner. The Java surface and the thrift field are separate commits, so you can take just the first one:

Pre-existing names like multicastToUser and getUserParagraphMap mean a delivery target or a map key, so I left those alone.

Comment on lines +40 to +42
// Execution owner. Null when the interpreter predates this field; see
// NotebookServer.onOutputAppend for how ownerless output is handled.
6: string user

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotebookServer.onOutputAppend is the Helium app output overload that this PR does not rename (the one taking appId). Ownerless output is dropped in onParagraphOutputAppend, so this pointer sends the reader to a different method.

The null half is already stated by the log line in that branch.

Suggested change
// Execution owner. Null when the interpreter predates this field; see
// NotebookServer.onOutputAppend for how ownerless output is handled.
6: string user
6: string user

Comment on lines +958 to +959
// The execution owner is fixed here, before any output is produced, so that every event
// emitted by this output carries the same owner regardless of what runs later.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of the three things this comment says are already in the code. The value is an argument to createInterpreterOutput, so it cannot be computed after the output exists, and the listener captures it as a final parameter exactly the way it captures noteId and paragraphId, neither of which carries such a comment.

What is left, that this is the execution owner, fits in a name:

private InterpreterContext convert(RemoteInterpreterContext ric) {
  return convert(ric, createInterpreterOutput(
      ric.getNoteId(), ric.getParagraphId(), executionOwnerOf(ric)));
}

private static String executionOwnerOf(RemoteInterpreterContext ric) {
  AuthenticationInfo authenticationInfo =
      AuthenticationInfo.fromJson(ric.getAuthenticationInfo());
  return authenticationInfo == null ? null : authenticationInfo.getUser();
}

Comment on lines +1780 to +1781
// An interpreter that predates the owner field leaves personalized output unaddressed,
// so it is dropped rather than sent to every reader of the note.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log line right below says the same thing.

The comments on the update and clear branches are different: they carry facts from outside this file (what checkpointOutput saves, and where a new user's copy is cloned from), so those are worth keeping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants