Skip to content

[fix][fn] Make exclusiveLeaderProducer volatile in FunctionMetaDataManager - #26046

Merged
lhotari merged 1 commit into
apache:masterfrom
SongOf:fix/function-metadata-leader-producer-volatile
Jun 17, 2026
Merged

[fix][fn] Make exclusiveLeaderProducer volatile in FunctionMetaDataManager#26046
lhotari merged 1 commit into
apache:masterfrom
SongOf:fix/function-metadata-leader-producer-volatile

Conversation

@SongOf

@SongOf SongOf commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

PR Description

Motivation

In FunctionMetaDataManager, the exclusiveLeaderProducer field serves a
double duty: a non-null value means this worker is the leader, a null value
means it is not. It is the sole signal the write path uses to decide whether
this worker may publish to the function metadata topic.

The field is written by acquireLeadership(), which is intentionally not
synchronized: it waits for the metadata tailer thread to drain
(tailer.stopWhenNoMoreMessages().get()), and holding the monitor there would
deadlock with the tailer thread, which calls the synchronized
processUpdate() / processDeregister(). The field is then read by the
synchronized methods updateFunctionOnLeader(), start() and
giveupLeadership().

Because the write in acquireLeadership() does not hold the monitor, there is
no happens-before edge between that write and the synchronized reads. Under the
Java Memory Model this permits a reader to observe a stale null after
leadership has actually been acquired, and updateFunctionOnLeader() would then
spuriously throw IllegalStateException("Not the leader"). The field is plain
(non-volatile), unlike the sibling cross-thread field lastMessageSeen, which
is already declared volatile.

Modifications

  • Declared FunctionMetaDataManager.exclusiveLeaderProducer as volatile.

This establishes the missing happens-before edge between the unsynchronized
write in acquireLeadership() and the synchronized reads, so the leadership
signal is always published safely. The change is limited to the field modifier;
no logic, locking, public API, or threading model is changed, and it is
consistent with how lastMessageSeen is already handled in the same class.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial correctness/hardening fix without dedicated test
coverage. The defect is a Java Memory Model visibility hazard that cannot be
reliably reproduced by a unit test: the reads happen inside synchronized
blocks (acquire fence + field reload) and the JVM runs on cache-coherent
hardware, so a failing reproduction cannot be constructed deterministically or
intermittently. The fix is verified by inspection against the JMM and is
covered by the existing FunctionMetaDataManagerTest for behavioral regression.

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

…aDataManager

exclusiveLeaderProducer is written by acquireLeadership(), which is
intentionally not synchronized (it waits for the metadata tailer thread to
drain, and holding the monitor there would deadlock with the tailer's
synchronized processUpdate/processDeregister). The field is then read by the
synchronized methods updateFunctionOnLeader(), start() and giveupLeadership()
as the sole "am I the leader?" signal.

Because the write happens without holding the monitor, there is no
happens-before edge to those synchronized reads, so the Java Memory Model
permits a reader to observe a stale null and spuriously throw "Not the leader".
Marking the field volatile establishes the missing happens-before edge, matching
how the sibling cross-thread field lastMessageSeen is already declared.
@lhotari lhotari added this to the 5.0.0-M2 milestone Jun 17, 2026
@lhotari lhotari changed the title [fix][functions] Make exclusiveLeaderProducer volatile in FunctionMet… [fix][fn] Make exclusiveLeaderProducer volatile in FunctionMet… Jun 17, 2026
@lhotari lhotari changed the title [fix][fn] Make exclusiveLeaderProducer volatile in FunctionMet… [fix][fn] Make exclusiveLeaderProducer volatile in FunctionMetaDataManager Jun 17, 2026
@lhotari
lhotari merged commit b8b1e2c into apache:master Jun 17, 2026
44 of 46 checks passed
lhotari pushed a commit that referenced this pull request Jun 22, 2026
…nager (#26046)

Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
(cherry picked from commit b8b1e2c)
lhotari pushed a commit that referenced this pull request Jun 22, 2026
…nager (#26046)

Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
(cherry picked from commit b8b1e2c)
sandeep-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2026
…nager (apache#26046)

Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
(cherry picked from commit b8b1e2c)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Aug 28, 2026
…nager (apache#26046)

Co-authored-by: maxlisongsong <maxlisongsong@didiglobal.com>
(cherry picked from commit b8b1e2c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants