Fix routing to pipelines wip - #19
Closed
johnnyshields wants to merge 3 commits into
Closed
Conversation
… is dynamically created at runtime, so routing to it is being done by deferred name-based routing which isn't working. This became FUBAR
Contributor
Author
|
Fixed by #24 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[WIP in Routing to pipelines. Issue seems to be that the EntranceStage is dynamically created at runtime, so routing to it is being done by deferred name-based routing which isn't working.
This became FUBAR
We should start over with a simpler implementation
Summary of the Routing to Nested Pipelines Issue
The Problem
When you route to a nested pipeline by name (e.g., to: [:process_fast]), there are two stages involved:
PipelineStage - Created at define-time, wraps the nested pipeline, added to parent DAG
EntranceStage - Created at runtime (in insert_entrance_distributor_for_inputs!), receives items inside the nested pipeline
The conflict: Router stages resolve names to Stage objects, then look up queues by those objects. But:
If PipelineStage doesn't have the name → routing can't find it
If EntranceStage doesn't have the name → can't be found by name
If both have the name → duplicate names in registry
Router has PipelineStage objects in @targets, but queues are keyed by EntranceStage objects
The Options
Option 1: PipelineStage unnamed, EntranceStage has pipeline name
✅ Clean registry (no duplicates)
❌ PipelineStage can't be found for routing (but it's in DAG)
❌ EntranceStage created too late for early routing resolution
Option 2: Both have name, alias in registry find_by_name
✅ Both can be registered and found
✅ Registry automatically redirects PipelineStage → EntranceStage
❌ Complex: duplicate names in registry, need aliasing logic
🔧 Currently partially implemented
Option 3: PipelineStage named, redirect in queue lookup instead
✅ Simple registry
✅ PipelineStage findable
❌ Need special case in build_stage_input_queues to redirect PipelineStage → EntranceStage
❌ Router still has wrong objects in @targets
Option 4: Create EntranceStage early (at pipeline init)
✅ Available immediately for routing
❌ Can't get queue - @input_queues not set until runtime (set by PipelineStage)
❌ Just tried this, failed because queue lookup happens before @input_queues is set
Recommendation
Option 2 with clean implementation:
Both PipelineStage and EntranceStage have the pipeline's name
Don't register PipelineStage by name (it's not routable)
Registry find_by_name returns EntranceStage
Queue lookup just works (EntranceStage in queues)
OR simpler: Fix Option 3 - Make build_stage_input_queues and routing handle PipelineStage → EntranceStage redirection.
Deeper Refactor Options
Yes, there are several architectural refactors that could eliminate this complexity entirely:
Option A: Eliminate PipelineStage - Make Pipeline a DAG node
Concept: Treat nested Pipeline objects as first-class DAG nodes, no wrapper needed.
Pros:
✅ No PipelineStage/EntranceStage confusion
✅ Direct routing: to: [:process_fast] finds the Pipeline
✅ Simpler: one object per nested pipeline
Cons:
🔧 Pipeline needs to implement Stage interface
🔧 DAG currently assumes Stage objects
🔧 Worker creation assumes Stage objects
POSSIBLY THE BEST
Option B: Eliminate EntranceStage - PipelineStage is the entrance
Concept: PipelineStage itself has the queue and acts as the entry point.
Pros:
✅ One stage per pipeline (PipelineStage)
✅ Clean routing: PipelineStage has name and queue
✅ No entrance/exit stage generation
Cons:
🔧 PipelineStage needs to manage forwarding logic
🔧 Nested pipeline entry points need special handling
🔧 Less clean separation between "wrapper" and "contents"
Option C: Two-phase routing resolution
Concept: Separate "routing targets" from "execution stages"
Pros:
✅ Clean separation of routing namespace vs execution
✅ Lazy resolution at runtime
✅ Can handle complex routing logic
Cons:
🔧 More complex registry
🔧 Two-phase system to understand
Option D: Make EntranceStage the primary, PipelineStage is metadata
Concept: PipelineStage exists only in parent DAG for ordering, not for routing/queues.
Pros:
✅ Clear separation: DAG uses PipelineStage, execution uses EntranceStage
✅ Minimal changes needed
Cons:
🔧 Still need redirection logic somewhere
My Recommendation
Option B (Eliminate EntranceStage) seems cleanest:
PipelineStage gets the pipeline name
PipelineStage has the input queue
PipelineStage internally manages nested pipeline execution
No separate entrance/exit stages needed
Routing just works: name → PipelineStage → queue exists
This would be a moderate refactor but would permanently solve the dual-stage problem.
Would you like me to implement Option B?