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.

Sign up for GitHub

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

Jump to bottom

Make report propagation more reliable #459

Merged
merged 2 commits into from
Sep 9, 2015
Merged

Make report propagation more reliable #459

merged 2 commits into from
Sep 9, 2015

Conversation

tomwilkie
Copy link
Contributor

Fixes #458, fixes #467

  • Make scope push the reports asynchronously
  • Backoff when report push fails

@tomwilkie tomwilkie self-assigned this Sep 7, 2015
@tomwilkie tomwilkie changed the title WIP Warn when generating the report takes too long. WIP Fixes Sep 7, 2015
@tomwilkie tomwilkie changed the title WIP Fixes Make report propagation more reliable Sep 8, 2015
@tomwilkie tomwilkie force-pushed the 458-blankety-blank branch 2 times, most recently from cfa32f7 to fbe513e Compare September 8, 2015 17:52
peterbourgon
peterbourgon reviewed Sep 9, 2015
View reviewed changes
report/id.go
"net"
"strings"
"sync"

cache "github.com/hashicorp/golang-lru"

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@tomwilkie tomwilkie removed their assignment Sep 9, 2015
@tomwilkie
Copy link
Contributor Author

The problem was we were pushing reports on the main probe thread, one-by-one. If one of the ips was unreachable, it would block the pushing of all others, and the probe would stall and the UI go blank. And tcp connection time out is quite long.

paulbellamy
paulbellamy reviewed Sep 9, 2015
View reviewed changes
xfer/publisher.go
@@ -83,12 +94,73 @@ func (p HTTPPublisher) Publish(rpt report.Report) error {
return nil
}

// Stop implements Publisher
func (p HTTPPublisher) Stop() {}

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@paulbellamy
Copy link
Contributor

When full, this seems to attempt to throw away new reports, and keep retrying the original one which failed. Don't we really only care about the latest report?

@tomwilkie
Copy link
Contributor Author

We don't retry the sending of any reports - why do you think it does?

@paulbellamy
Copy link
Contributor

You are right, misread the backoff waiting bit.

@paulbellamy
Copy link
Contributor

Aside from making the http handler cancel the existing request (if that is important), LGTM.

tomwilkie added a commit that referenced this pull request Sep 9, 2015
@tomwilkie
Merge pull request #459 from weaveworks/458-blankety-blank
57aaf4b 
Make report propagation more reliable
@tomwilkie tomwilkie merged commit 57aaf4b into master Sep 9, 2015
@tomwilkie tomwilkie deleted the 458-blankety-blank branch September 9, 2015 12:49
@tomwilkie tomwilkie mentioned this pull request Sep 9, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need to bring back connection backoff (when probe pushes to app) Scope view intermittently goes blank
3 participants
@tomwilkie @paulbellamy @peterbourgon