Skip to content

[RFC] Introduce data_publisher - #10426

Merged
frazze-jobb merged 2 commits into
erlang:masterfrom
zzydxm:data_publisher
Apr 2, 2026
Merged

[RFC] Introduce data_publisher#10426
frazze-jobb merged 2 commits into
erlang:masterfrom
zzydxm:data_publisher

Conversation

@zzydxm

@zzydxm zzydxm commented Dec 1, 2025

Copy link
Copy Markdown
Contributor

Generalized data publishing logic of pg to a behavior module, so that it better supports version upgrade, and can be used for other data publishing scenarios.

I also changed the message sent between pg gen_servers to be a general update instead of tuples starts with join/leave. This makes it able to support more efficient APIs.

This replaces PR of #7609

I have a small change that makes pg:demonitor only return ok instead of ok | false, this is revertible but I believe no one is using the 'false' return

@github-actions

github-actions Bot commented Dec 1, 2025

Copy link
Copy Markdown
Contributor

CT Test Results

    3 files     85 suites   1h 10m 51s ⏱️
1 914 tests 1 497 ✅ 417 💤 0 ❌
2 189 runs  1 712 ✅ 477 💤 0 ❌

Results for commit 2d68cbd.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@max-au

max-au commented Dec 1, 2025

Copy link
Copy Markdown
Contributor

Tastes like gen_pubsub to me!
Now, while I like the general idea, when I thought of it, I'd expect it to be significantly more generic. To a degree of having local process registry - therefore not calling net_kernel:monitor_nodes at all. This is fairly common request to have "pg but local to a specific node".

@zzydxm

zzydxm commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

Now, while I like the general idea, when I thought of it, I'd expect it to be significantly more generic. To a degree of having local process registry - therefore not calling net_kernel:monitor_nodes at all. This is fairly common request to have "pg but local to a specific node".

Oh do you mean start the gen_server without allowing it to have any cross-node communication? I can think how to add this functionality

@IngelaAndin IngelaAndin added the team:VM Assigned to OTP team VM label Dec 8, 2025

@frazze-jobb frazze-jobb 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.

Great job, I like it. I have some documentation, and naming comments, that we can discuss.
I was able to trigger a crash when trying this out, so that needs fixing. I will maybe try some more, to see if I can find something untested.

Comment thread lib/kernel/src/data_publisher.erl
Comment thread lib/kernel/src/data_publisher.erl Outdated
Comment thread lib/kernel/test/pg_SUITE.erl
Comment thread lib/kernel/src/data_publisher.erl Outdated
Comment thread lib/kernel/src/data_publisher.erl Outdated
Comment thread lib/kernel/src/data_publisher.erl Outdated
Comment thread lib/kernel/src/data_publisher.erl
Comment thread lib/kernel/src/data_publisher.erl Outdated
@zzydxm

zzydxm commented Jan 30, 2026

Copy link
Copy Markdown
Contributor Author

updated with some doc and bug fixes
I think custom_state() maybe better than data_cache() or storage(), how do you think?

@frazze-jobb frazze-jobb 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.

I don't like custom_state,

How about data -> local_state
and storage -> global_view?

Motivation:
Global view implies that what you see is a projection or a materialized summary of the whole cluster. It signals to the developer that:
It contains data from multiple places.
It might be slightly out of sync (a "view" can be stale).
It's not necessarily an exact copy of the local_state (data).

Comment thread lib/kernel/src/data_publisher.erl Outdated
@zzydxm

zzydxm commented Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

Sure I can do the change, just a note that global_view can contain more things than a "view": In pg it is the monitors of the local pids

% Init custom publisher state, which should include a local cache storing all nodes' data,
% and potentially some internal state.
% For example, it can also store some monitors, so that it can automatically do some data
% updates when monitored events happened, see the optional translate_message callback

@frazze-jobb

Copy link
Copy Markdown
Contributor

Yes, this behavior can be used in that way, but I imagine that there should be clean cuts between state and view. Maybe the monitors could be part of the local state, but not affect the view by calling update in update_view_and_notify. But it works now and I dont think you need to make the change on pg. But if you want you can try.

@zzydxm

zzydxm commented Feb 11, 2026

Copy link
Copy Markdown
Contributor Author

I updated the PR to have type name change to global_view and local_data
I tried to separate global_view to global_view and custom_state but then it looks non intuitive, will keep it for now

@frazze-jobb frazze-jobb added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Mar 9, 2026

@frazze-jobb frazze-jobb 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.

I'm happy with this, minor problem with licence header, and a failing testcase: kernel_SUITE:app_test

Comment thread lib/kernel/src/data_publisher.erl Outdated
@zzydxm

zzydxm commented Mar 12, 2026

Copy link
Copy Markdown
Contributor Author

great, updated with the fix, thanks!

@frazze-jobb frazze-jobb added the testing currently being tested, tag is used by OTP internal CI label Mar 12, 2026
@bjorng bjorng removed the testing currently being tested, tag is used by OTP internal CI label Mar 19, 2026
@bjorng

bjorng commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

The license header check fails. Removed from our daily builds.

@zzydxm

zzydxm commented Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

all checks passing now

@bjorng bjorng added the testing currently being tested, tag is used by OTP internal CI label Mar 20, 2026
@bjorng

bjorng commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Thanks!

Please squash your commits. See Writing good commit messages.

@zzydxm

zzydxm commented Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

Sure, updated

Generalized data publishing logic of pg to a behavior module, so that
it better supports version upgrade, and can be used for other data
publishing scenarios.

The message sent between pg gen_servers to be a general update instead
of tuples starts with join/leave. This makes it able to support more
efficient APIs.
@frazze-jobb
frazze-jobb merged commit a579e70 into erlang:master Apr 2, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants