Merge lp:~nikwen/account-polld/directly-poll-with-new-account-data-fix into lp:~ubuntu-push-hackers/account-polld/trunk

Proposed by Niklas Wenzel on 2014-10-20
Status: Merged
Approved by: John Lenton on 2015-03-20
Approved revision: 103
Merged at revision: 112
Proposed branch: lp:~nikwen/account-polld/directly-poll-with-new-account-data-fix
Merge into: lp:~ubuntu-push-hackers/account-polld/trunk
Diff against target: 21 lines (+3/-0)
1 file modified
cmd/account-polld/main.go (+3/-0)
To merge this branch: bzr merge lp:~nikwen/account-polld/directly-poll-with-new-account-data-fix
Reviewer Review Type Date Requested Status
Niklas Wenzel (community) Approve on 2014-12-02
John Lenton 2014-10-20 Needs Fixing on 2014-10-31
Review via email: mp+238901@code.launchpad.net

Description of the Change

Existing accounts which have new account data will now directly poll the server. Otherwise it would wait 5 minutes (until Poll() is called again), recognize it failed before setting the new account data (penaltyCount != 0) and wait 5 minutes again until polling finally happens in the next invokation of Poll().

To post a comment you must log in.
Niklas Wenzel (nikwen) wrote :

This possibly also fixes a multithreading issue which may occur in the monitorAccounts() function as without this fix it wouldn't wait for the Poll() calls in "case data := <-watcher.C:".

Niklas Wenzel (nikwen) wrote :

Ok, please don't merge this yet. I found an error in the logs. It seems like there is some problem in the code which starts the authentication process again. It looks like this happened because the reference to the variable 'account' got updated in the meantime. I'll look into this tomorrow.

2014/10/20 16:02:38 Poll for account 2 has failed: Token expired
2014/10/20 16:02:38 Ending poll for account 2
2014/10/20 16:02:38 Poll for account 3 has failed: Token expired
2014/10/20 16:02:38 Ending poll for account 3
2014/10/20 16:02:40 New account data for existing account with id 2
2014/10/20 16:02:40 Starting poll for account 2
2014/10/20 16:02:40 Polling account 2
2014/10/20 16:02:41 Account 2 has 0 updates to report
2014/10/20 16:02:41 Poll for account 2 was successful
2014/10/20 16:02:41 Ending poll for account 2
2014/10/20 16:02:47 Creating account with id 2 for com.ubuntu.developer.webapps.webapp-gmail_webapp-gmail
2014/10/20 16:02:47 gmail plugin 2: last state loaded from storage
2014/10/20 16:02:47 Starting poll for account 2
2014/10/20 16:02:47 Polling account 2
2014/10/20 16:02:48 Account 2 has 0 updates to report
2014/10/20 16:02:48 Poll for account 2 was successful
2014/10/20 16:02:48 Ending poll for account 2
2014/10/20 16:03:08 Creating account with id 2 for com.ubuntu.developer.webapps.webapp-gmail_webapp-gmail
2014/10/20 16:03:08 gmail plugin 2: last state loaded from storage
2014/10/20 16:03:08 Starting poll for account 2
2014/10/20 16:03:08 Polling account 2
2014/10/20 16:03:09 Account 2 has 0 updates to report
2014/10/20 16:03:09 Poll for account 2 was successful
2014/10/20 16:03:09 Ending poll for account 2

Niklas Wenzel (nikwen) wrote :

The solution for this problem is described here: https://code.google.com/p/go-wiki/wiki/CommonMistakes#Using_Closures_with_Goroutines

You see, it's been the first time I've ever used Go.

Niklas Wenzel (nikwen) wrote :

I'll have this running in the background for some time today to see if the fix resolves the issue. It should, however, do in my opinion.

Niklas Wenzel (nikwen) wrote :

I can confirm that the issue is fixed now. :)

John Lenton (chipaca) wrote :

This is a rather big hammer for the fix you're doing. Let me dig into it a bit.

review: Needs Fixing
Niklas Wenzel (nikwen) :
review: Needs Fixing
Niklas Wenzel (nikwen) wrote :

Sorry, I changed the status on the wrong merge proposal.

Niklas Wenzel (nikwen) wrote :

Ok, now it's ready to be merged again. :)

Niklas Wenzel (nikwen) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/account-polld/main.go'
2--- cmd/account-polld/main.go 2014-10-02 13:58:49 +0000
3+++ cmd/account-polld/main.go 2014-12-02 20:11:53 +0000
4@@ -1,6 +1,7 @@
5 /*
6 Copyright 2014 Canonical Ltd.
7 Authors: Sergio Schvezov <sergio.schvezov@canonical.com>
8+ Niklas Wenzel <nikwen.developer@gmail.com>
9
10 This program is free software: you can redistribute it and/or modify it
11 under the terms of the GNU General Public License version 3, as published
12@@ -107,7 +108,9 @@
13 if account, ok := mgr[data.AccountId]; ok {
14 if data.Enabled {
15 log.Println("New account data for existing account with id", data.AccountId)
16+ account.penaltyCount = 0
17 account.updateAuthData(data)
18+ account.Poll(false)
19 } else {
20 account.Delete()
21 delete(mgr, data.AccountId)

Subscribers

People subscribed via source and target branches