Skip to content

Remove InputGuard, refactor test components without it #64

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

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

akokhanovskyi
Copy link
Contributor

Rationale: InputGuard is unsafe to use; components for test provide a bad example.
Explanation: The key problem is that after a channel is closed, it never blocks reads. The existing implementation leads to a dead select loop, which eventually exits when select randomly chooses another execution branch. Updated component implementations provide a more accurate example of handling closed channels in go.
Additionally, keeping a map of inputs is expensive and totally unnecessary, as it can be seen from the updated impl.
Finally, if we were to preserve InputGuard-s for some reason, the implementation would have to be changed for safety. Consider what would happen if you accidentally Complete()-d a non-existing port. What would happen if you accidentally Complete()-d the same port twice?

@trustmaster
Copy link
Owner

@akokhanovskyi I agree InputGuard is error-prone and probably it isn't straight-forward either.

Now that we are at it, I'd like to ask for your ideas about solving the original problem that InputGuard was targeting. The problem is not handling closed channels, the problem is simplifying handling of multiple channels in the same process. InputGuard was my first experiment in this area.

The most typical situation is the following. We have some component C which has multiple inports, e.g. In1, In2, In3. Behaviour of the component is: read inputs in1, in2, in3 on all of its inputs, and then apply some function f(in1, in2, in3) to get the result. Typically, result is then sent to its output. Once all of its input channels are closed, the process terminates.

Components like this are so common, that doing manual select logic every time is very repetitive. Ideally, there should be some easy to use API for this. And ideally, it shouldn't use reflection, because using reflection on every activation of a process is expensive (this was the main critique for GoFlow v0).

@akokhanovskyi
Copy link
Contributor Author

Right, staying low on reflection makes perfect sense. So far, the best option that comes to my mind is to provide some code generation tool that would reduce the amount of boilerplate to be written. I am not ready to comment on the functionality or usefulness of such a tool, though.

On the other hand, could a few small examples illustrating typical use cases fit the bill as the MVP? I could use a few of those myself when starting with goflow :)

@trustmaster trustmaster merged commit 288dea0 into trustmaster:master Oct 26, 2020
@trustmaster
Copy link
Owner

I tried to think of a solution on the weekend but could not come up with anything that would work without generics or reflection. I guess we can do with heavy boilerplate for now.

As per examples, all I have currently is the tests and #53 which I'm using as a playground. This is all I can offer at this point, because my time to work on this project is very limited.

@akokhanovskyi akokhanovskyi deleted the noinputguard branch October 26, 2020 20:02
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.

2 participants