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

Send incremental reports probe->app #985

Closed
2opremio opened this issue Feb 18, 2016 · 4 comments
Closed

Send incremental reports probe->app #985

2opremio opened this issue Feb 18, 2016 · 4 comments
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality performance Excessive resource usage and latency; usually a bug or chore
Milestone

Comments

@2opremio
Copy link
Contributor

The uncompressed report sizes are very large, ~1MB each. This puts a lot of pressure on the codecs and causes a lot of traffic.

See #854 (comment)

We could send reports incrementally (i.e. full report + diffs) to improve this.

This was referenced Feb 18, 2016
@peterbourgon
Copy link
Contributor

To send an incremental report, the probe must know what the app "already knows" in order to calculate a diff. That means the current (stateless) POST method of getting reports to apps is insufficient, and we'll need to maintain a persistent connection. Given the Go HTTP server keeps the connection around between POSTs currently, this is probably fine from an ops POV. But for the cloud offering, for example, we will end up creating a persistent connection from every node in the customer infrastructure to whatever.weave.works. Do we think that's acceptable?

I suggest WebSockets, since we're already doing something exactly like this between the app and the web UI. Upon connection, send the complete report. Thereafter, send deltas. (Maybe every ~minute, dump the complete report again, as a sync.) If the connection ever dies, throw away your state and start again. Maybe can even reuse some code.

If you do this, don't remove the existing POST functionality. It's still very useful for debugging, and the various test and fix probes.

@tomwilkie
Copy link
Contributor

This will also need some thought wrt multi tenancy. Some sort of
persistent connection (is a websocket) does indeed seem the way to go.

On Thursday, 18 February 2016, Peter Bourgon notifications@github.com
wrote:

To send an incremental report, the probe must know what the app "already
knows" in order to calculate a diff. That means the current (stateless)
POST method of getting reports to apps is insufficient, and we'll need to
maintain a persistent connection. Given the Go HTTP server keeps the
connection around between POSTs currently, this is probably fine. But for
the cloud offering, for example, we will end up creating a persistent
connection from every node in the customer infrastructure to
whatever.weave.works. Do we think that's acceptable?

I suggest WebSockets, since we're already doing something exactly like
this between the app and the web UI. Upon connection, send the complete
report. Thereafter, send deltas. (Maybe every ~minute, dump the complete
report again, as a sync.) If the connection ever dies, throw away your
state and start again. Maybe can even reuse some code.

If you do this, don't remove the existing POST functionality. It's still
very useful for debugging, and the various test and fix probes.


Reply to this email directly or view it on GitHub
#985 (comment).

@2opremio
Copy link
Contributor Author

To send an incremental report, the probe must know what the app "already knows" in order to calculate a diff.

Not in general due to the CRDT semantics of reports (there are some exceptions to this like metrics we need to figure out, but we can decide not to send those incrementally for now).

We would though need to have strict guarantees on full reports being received periodically which is much easier to obtain than keeping state about what the app knows.

POST method of getting reports to apps is insufficient, and we'll need to maintain a persistent connection

As per the comment above I don't think that's true.

@2opremio 2opremio added the performance Excessive resource usage and latency; usually a bug or chore label Jun 30, 2016
@2opremio 2opremio modified the milestone: July2016 Jun 30, 2016
@rade rade added the chore Related to fix/refinement/improvement of end user or new/existing developer functionality label Jul 4, 2016
@2opremio 2opremio modified the milestones: July2016, August2016 Aug 2, 2016
@2opremio 2opremio mentioned this issue Aug 13, 2016
3 tasks
@rade rade modified the milestones: August2016, September2016 Aug 16, 2016
@bboreham
Copy link
Collaborator

Done in #3677

the probe must know what the app "already knows" in order to calculate a diff

As described in the PR, this turned out to be unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality performance Excessive resource usage and latency; usually a bug or chore
Projects
None yet
Development

No branches or pull requests

5 participants