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

Calculate e_p values when messages are broadcasted to individual clients #66

Open
clandry94 opened this issue Jan 17, 2018 · 0 comments
Assignees
Labels

Comments

@clandry94
Copy link
Member

clandry94 commented Jan 17, 2018

What's Wrong?

Currently, we are calculating the e_p value when a payload is created in the application layer of carrot as seen here. The issue with this is that if more than two people are connected to the server, then the e_p value will be invalid since e_p can only be calculated for the first secondary device that connects to the server. This results in every client that isn't the first secondary device to receive the e_p value for the first secondary device.

Potential Fixes

I've been thinking of a few ways to fix this and I'll go through them in order of the most difficult approach to the fastest, simplest approach:

Rewrite request/response handling to use a context for encapsulation

This is the most difficult way to approach this and I've been working on it a bit before the new year in the calc_ep_all_messages branch. Alas, the break was long and comfortable and I lost track of what I was doing there. In essence, the approach taken was to provide something similar to the Buffalo Context and contain everything related to every request in its own context. This way, we can just call something like calculateE_P() right before the message is broadcasted. This approach is going to require the most amount of work, but will be the cleanest and should be approached this way eventually.

Do the above, but only for the modules directly surrounding the e_p calculation and broadcasting

Almost exactly the same as the above, we could create a factory to build a context but build the context right before control of the request is handed up to the application layer. This will be faster to develop and a good trial to see how it works at the cost of increasing code complexity and is harder to debug.

Ignore switching to contexts all together, for now, and just leave response building to the broadcaster

Right now the e_p is calculated in here like was mentioned above. This happens when the response is built at the application layer here. This begs the question of:

Should the application layer even be building the response? Why should the application layer and developer care about serialized binary data?

To fix this, we could refactor the response building such that it is inaccessible to the developer. As a result, we can store the entire response, unserialized, in the OutMessage data structure. Doing so allows us to calculate the e_p value and serialize the response somewhere around here.

This is the simplest and easiest approach at the cost of not solving any of the decoupled response/request issues that would be solved by the first two. Yet, considering that we should try to keep this change as atomic as possible due to the potentially finicky nature of calculating the e_p value it may be the best for now.

Finally, this bug is blocking the progression of #62 so it is complete more feature dev can take place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant