Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure IPs are only sent once when sending to ArrayPort on a subgraph #41

Closed
wants to merge 2 commits into from

Conversation

kenhkan
Copy link
Contributor

@kenhkan kenhkan commented Aug 19, 2012

When sending to a subgraph, the same IP is sent to the subgraph the same number of times as how many processes are attached to an open ArrayPort. Something like:

subgraph.fbp:

Merge(Merge) OUT -> IN Output(Output) 

main_graph.fbp:

'subgraph.fbp' -> IN Graph(Graph)
'a' -> MERGE.IN Graph()
'b' -> MERGE.IN Graph()
'c' -> MERGE.IN Graph()

The above should print:

a
b
b
c
c
c

This seems to be due to using ArrayPort-to-ArrayPort attachment to make a link between the graph and the subgraph. Sending the IP only to the first socket solves the problem as all the sockets point to the same process in the subgraph anyway.

@travisbot
Copy link

This pull request passes (merged ff09619 into 5f4fcfa).

@kenhkan
Copy link
Contributor Author

kenhkan commented Aug 20, 2012

Just a clarification. The above currently prints:

a
b
b
c
c
c

where it should print:

a
b
c

@travisbot
Copy link

This pull request passes (merged 7f68b38 into 5f4fcfa).

@kenhkan
Copy link
Contributor Author

kenhkan commented Aug 25, 2012

I have this crazy thought... Would this problem persist if the whole noflo is implemented asynchronously? Say:

'SomeInitial' -> IN A(Something) OUT -> IN B(Graph) OUT -> IN C(SomethingElse)

In the current implementation, 'SomeInitial' is sent to A and then sent to B immediately. If data flow is async, wouldn't B setting up be run before 'SomeInitial' is passed to it, making the ports to be available for attachment between A and B?

@kenhkan
Copy link
Contributor Author

kenhkan commented Aug 27, 2012

See #42

@bergie
Copy link
Member

bergie commented Aug 28, 2012

Thanks for the PR! I'm mostly offline this week in Istanbul, so I'll be able to look at these in details next week

@bergie
Copy link
Member

bergie commented Oct 1, 2012

@kenhkan sorry about taking my time with these. This pull req doesn't apply cleanly any longer, unfortunately. Can you rebase and push so it gets updated?

@kenhkan
Copy link
Contributor Author

kenhkan commented Oct 19, 2012

@bergie Humorously the same thing happened to me. I was in Vietnam for several weeks so I'm sorry that I couldn't comment any earlier.

#41 should not be an issue if #42 is fixed. It's two solutions to the same problem. Well, #42 is a more complete solution. I'm closing this now.

@kenhkan kenhkan closed this Oct 19, 2012
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.

None yet

3 participants