He-Pin commented on issue #2860:
URL: https://github.com/apache/pekko/issues/2860#issuecomment-4792956839
Review update after re-checking current `main` and the existing issue
comments.
I would refine the remaining roadmap as follows:
1. Treat `Sink.asPublisher(fanout = true)` as a small follow-up
investigation, not as a redo of the already merged runtime migration.
- #2874 moved the fanout publisher runtime to
`FanoutPublisherBridgeStage`.
- Current `Sink.asPublisher(true)` still constructs
`FanoutPublisherSink`, which is a `SinkModule` wrapper.
- `FanoutPublisherSink.create` then materializes
`Source.asSubscriber.toMat(Sink.fromGraph(new FanoutPublisherBridgeStage))`
internally.
- So the runtime fanout behavior is GraphStage-backed, but the
construction/materialization path still goes through the old `SinkModule` plus
`Source.asSubscriber` boundary.
2. A focused PR could explore making `Sink.asPublisher(fanout = true)`
return the bridge stage directly.
- Goal: preserve the existing materialized `Publisher`, fanout semantics,
input-buffer behavior, subscription timeout behavior, active/late subscriber
behavior, and abrupt-shutdown signaling.
- Possible cleanup if successful: remove `FanoutPublisherSink` if it has
no remaining users.
- Keep `Sink.asPublisher(fanout = false)` out of scope, since that still
has different single-subscriber `VirtualPublisher` behavior.
3. Keep TLS cleanup separate.
- #2878 has added the TLS GraphStage engine, but the legacy actor path is
still the default in config.
- `TLSActor` should stay until downstream direct users such as pekko-http
are migrated.
- After downstream usage is gone, switch the default to `graph-stage`
with a transition fallback, then remove `TlsModule`, `TlsModuleIslandTag`,
`TlsModulePhase`, `TLSActor`, and only then prune now-unused `FanIn` / `FanOut`
/ `Transfer` substrate if grep, compile, tests, and MiMa agree.
4. Leave `SourceModule`, `SinkModule`, and `ProcessorModulePhase` as a later
Reactive Streams boundary cleanup track.
- They are boundary plumbing, not normal operators.
- They should not be bundled with TLS deletion.
- The `fanout = true` wrapper exploration above is a good narrow first
step toward shrinking that area.
Suggested validation for the `fanout = true` follow-up:
- `stream / Test / testOnly
org.apache.pekko.stream.impl.FanoutPublisherBehaviorSpec`
- `stream / Test / testOnly
org.apache.pekko.stream.scaladsl.PublisherSinkSpec`
- `stream / Test / testOnly org.apache.pekko.stream.scaladsl.SinkSpec`
- `stream / Test / testOnly org.apache.pekko.stream.scaladsl.FlowSpec`
- `stream / Test / testOnly org.apache.pekko.stream.impl.TimeoutsSpec`
- relevant `stream-tests-tck` fanout publisher coverage
- `stream / Test / compile`
- `stream / mimaReportBinaryIssues`
This keeps the next slice small: first remove the remaining `fanout = true`
SinkModule wrapper if behavior stays identical, then continue the larger TLS
and Reactive Streams boundary cleanup tracks separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
