| niemeyer | mthaddon: ping | 13:41 |
|---|---|---|
| mthaddon | niemeyer: hi | 13:41 |
| niemeyer | mthaddon: Hey man | 13:41 |
| niemeyer | mthaddon: We have our first bug to investigate | 13:41 |
| niemeyer | mthaddon: For some weird reason the charm at /charm/oneiric/mysql is returning 0 bytes | 13:42 |
| niemeyer | mthaddon: Theoretically, this should never happen since the charm is only linked into its final destination at the end of the importing process, which means any errors in between should leave it out | 13:43 |
| niemeyer | mthaddon: Oh, wow.. it looks like other charms are empty too | 13:44 |
| niemeyer | mthaddon: e.g. oneiric/jenkins | 13:45 |
| niemeyer | mthaddon: Can we debug this? | 13:45 |
| mthaddon | niemeyer: sure, can you give me a few mins - have another issue I'm looking at just now | 13:46 |
| niemeyer | Sure | 13:46 |
| mthaddon | niemeyer: okay - checking the logs | 13:50 |
| niemeyer | mthaddon: There's more than one in that state, so it's not a one-off event | 13:51 |
| mthaddon | niemeyer: nothing interesting in the web app logs - checking charmload | 13:52 |
| niemeyer | mthaddon: Please also run this later: db.charms.find({"urls": "cs:oneiric/jenkins"}) | 13:52 |
| niemeyer | mthaddon: In the juju db | 13:53 |
| mthaddon | niemeyer: https://pastebin.canonical.com/63655/ | 13:53 |
| niemeyer | mthaddon: Oops.. where's the size? | 13:55 |
| mthaddon | juju0.453125GB | 13:55 |
| mthaddon | is that what you mean? | 13:55 |
| niemeyer | mthaddon: Nope.. I mean I don't see the size in that json output | 13:56 |
| niemeyer | mthaddon: Please try this now: db.charms.find({"urls": "cs:oneiric/wordpress"}) | 13:56 |
| mthaddon | niemeyer: https://pastebin.canonical.com/63657/ | 13:57 |
| niemeyer | mthaddon: see the "size" there? | 13:57 |
| niemeyer | mthaddon: This is the bug that would happen if the database was loaded with the charmload prior to that update we did | 13:58 |
| niemeyer | mthaddon: The reason for dumping the database was avoiding this bug | 13:58 |
| mthaddon | niemeyer: ok, so we need to dump it again, confirm all cronjobs are stopped, and then start again? | 13:59 |
| niemeyer | mthaddon: No, now it's too late.. we don't want to break stuff people are using now | 14:00 |
| niemeyer | mthaddon: I'll do what I should have done in the first place and write an upgrader | 14:00 |
| niemeyer | mthaddon: My laziness clearly didn't pay off | 14:00 |
| mthaddon | okey doke | 14:00 |
| niemeyer | mthaddon: okay | 14:22 |
| niemeyer | mthaddon: Here is the fix: | 14:22 |
| niemeyer | mthaddon: https://pastebin.canonical.com/63660/ | 14:22 |
| mthaddon | niemeyer: is there any kind of logging/transactional status to this? just wondering what output you'll want to see to confirm it's done the right thing | 14:23 |
| niemeyer | mthaddon: No transactions.. you can exit on the first iteration of the loop if you want to be super sure | 14:24 |
| niemeyer | mthaddon: Hold on, will provide it | 14:24 |
| niemeyer | Ah, quit, not exit | 14:27 |
| mthaddon | yeah, quit() | 14:28 |
| niemeyer | mthaddon: https://pastebin.canonical.com/63664/ | 14:28 |
| niemeyer | mthaddon: Cool | 14:28 |
| niemeyer | mthaddon: This should tell you the first item it fixed, and give you a chance to go look | 14:28 |
| mthaddon | niemeyer: Fixing cs:~4-ja-d/oneiric/appflower | 14:29 |
| mthaddon | niemeyer: how do we verify that, or is that good enough? | 14:29 |
| niemeyer | mthaddon: Cool, if you do the find we had above now, with this url, it should show the entry with a size | 14:29 |
| niemeyer | mthaddon: Also, in addition to the find, please use .next() at the end | 14:29 |
| niemeyer | mthaddon: It will give you better formatted output | 14:29 |
| niemeyer | mthaddon: db.charms.find({urls: "cs:~4-ja-d/oneiric/appflower"}).next() | 14:30 |
| niemeyer | more precisely.. | 14:30 |
| mthaddon | niemeyer: https://pastebin.canonical.com/63666/ | 14:31 |
| * mthaddon sees size there, suspects that's a good thing | 14:31 | |
| niemeyer | mthaddon: Yep, looks good.. intact, and has a size which probably matches the actual file | 14:31 |
| mthaddon | ok, going ahead with the original paste | 14:32 |
| niemeyer | mthaddon: Hold on a second, though.. | 14:32 |
| mthaddon | holding | 14:32 |
| niemeyer | mthaddon: I'll download the actual charm | 14:32 |
| mthaddon | k | 14:32 |
| niemeyer | mthaddon: https://pastebin.canonical.com/63667/ | 14:33 |
| mthaddon | cool cool! | 14:33 |
| mthaddon | so go ahead with the first paste? | 14:34 |
| niemeyer | mthaddon: Good to go.. | 14:34 |
| mthaddon | k | 14:34 |
| mthaddon | niemeyer: and done | 14:34 |
| niemeyer | mthaddon: Super, please save the output in case something goes terribly wrong.. unlikely, but whatever | 14:34 |
| mthaddon | niemeyer: https://pastebin.canonical.com/63669/ | 14:34 |
| niemeyer | mthaddon: Superb, it's in the pastebin for eternity | 14:35 |
| niemeyer | mthaddon: Thanks for the help | 14:35 |
| mthaddon | yep :) | 14:35 |
| mthaddon | sure, np | 14:35 |
| niemeyer | mthaddon: Btw, do we have access from the charmd process to perform outside queries? | 14:46 |
| niemeyer | mthaddon: HTTP queries, more precisely | 14:46 |
| mthaddon | niemeyer: do you mean are the charmd processes exposed directly to the world? no, just through apache -> haproxy | 14:48 |
| mthaddon | apache -> haproxy -> charmd, I mean | 14:48 |
| niemeyer | mthaddon: No, I mean the opposite | 14:48 |
| niemeyer | mthaddon: Can I query *from* the charmd process into the world | 14:48 |
| mthaddon | niemeyer: all our servers have egress firewalls - if you want to connect to specific things we can make exceptions though | 14:49 |
| niemeyer | mthaddon: Cool, it's a specific thing indeed.. I'm pondering about collection of stats | 14:49 |
| mthaddon | k | 14:49 |
| niemeyer | TheMue: ping | 14:58 |
| TheMue | niemeyer: pong | 14:58 |
| niemeyer | TheMue: Heya | 14:58 |
| niemeyer | TheMue: How're things going there? | 14:58 |
| niemeyer | TheMue: How was the presentation? | 14:58 |
| TheMue | niemeyer: has been fine, the audience has been very interested. i only talked too long. ;) | 14:59 |
| niemeyer | TheMue: Nice! | 14:59 |
| TheMue | niemeyer: i have to fix three little typos in the slides and then they will be published | 14:59 |
| niemeyer | TheMue: COol | 15:00 |
| TheMue | niemeyer: but next time i'm keeping the pure talk shorter and concentrate more on a practical part. | 15:00 |
| niemeyer | TheMue: It's easy to go overboard which such a wide topic | 15:00 |
| TheMue | niemeyer: indeed, there is so much you can say about a language like go. and many people didn't know anything about concurrency or higher-order functions | 15:01 |
| niemeyer | TheMue: What was the meeting core topic? Is it a Linux User Group? | 15:02 |
| TheMue | niemeyer: but i've been asked to talk at other GTUGs (will be called GDG - Google Developer Group - in future) | 15:02 |
| TheMue | niemeyer: a Google User Group | 15:02 |
| niemeyer | TheMue: Hmm.. curious | 15:03 |
| niemeyer | TheMue: A group of users that ... ? | 15:03 |
| niemeyer | Use Google? :-) | 15:03 |
| TheMue | niemeyer: to be more special, a Google technology User Group. android, google apis, dart, go, ... | 15:03 |
| niemeyer | TheMue: Weird.. feels so open | 15:04 |
| niemeyer | Although I guess a LUG is equally open | 15:04 |
| niemeyer | TheMue: What about juju land? | 15:05 |
| TheMue | niemeyer: here i'm continuing with real watchers and wait for your lgtm | 15:06 |
| niemeyer | TheMue: Sounds good.. I should dedicate some good amount of time to reviews today | 15:07 |
| niemeyer | TheMue: How's the real watchers stuff going? | 15:07 |
| TheMue | niemeyer: using the watcher package helps, but i had to extend it a bit, you see it in the proposed service config watcher | 15:08 |
| niemeyer | TheMue: Ok | 15:08 |
| niemeyer | TheMue: have you been able to push things forward, or are you blocked? | 15:08 |
| TheMue | niemeyer: if a pipelined goroutine needs to signal an error, it needs a way to tell it to the watcher. | 15:08 |
| niemeyer | TheMue: Isn't that what Stop is for? | 15:09 |
| TheMue | niemeyer: i would say, like roger, that the config watcher is fine. and so i'm continuing based on that (implementing further watches the same way using a new branch based on the config watch branch) | 15:10 |
| niemeyer | TheMue: It doesn't have to be based on if there's no real dependency between them | 15:11 |
| TheMue | niemeyer: Kill() is for signalling errors | 15:11 |
| niemeyer | TheMue: Within the watcher.. something that happens out of the watcher.*Watcher isn't responsibility of that type | 15:11 |
| TheMue | niemeyer: Stop() is the a client/user of a watch can tell that it isn't interested anymore | 15:12 |
| niemeyer | TheMue: Exactly.. and if an error happens outside of that watcher, that's what should be done | 15:12 |
| niemeyer | TheMue: If you're feeling a need to Kill the watcher from outside, some abstraction is being broken somewhere | 15:13 |
| niemeyer | TheMue: I'll be happy to have a look at the branch after lunch | 15:13 |
| TheMue | niemeyer: it's a pipelined goroutine, in case of the config node the data may be invalid (yaml error), but i want to pass this information to the client of the watch. so i Kill() with that error. and the client, calling Stop(), get's this error returned. | 15:14 |
| niemeyer | TheMue: Exactly, that's not right | 15:14 |
| niemeyer | TheMue: You're using the watcher to pass errors for something it has no idea about | 15:14 |
| niemeyer | TheMue: Bad abstraction | 15:14 |
| niemeyer | TheMue: The watcher within the watcher package watches.. it's not supposed to be a conduit for errors of someone else | 15:15 |
| niemeyer | Anyway, really have to step out for lunch.. back in a moment | 15:15 |
| TheMue | niemeyer: the pipelined goroutine needs to tell the watcher 'stop working, there is an error'. | 15:16 |
| niemeyer | TheMue: No, it actually doesn't.. have you ever heard of a function called fkill in C? | 16:00 |
| niemeyer | TheMue: You call it like this: fkill(fd, "My custom error message") | 16:00 |
| niemeyer | TheMue: So that when you do fclose(fd), you can the error back | 16:00 |
| niemeyer | TheMue: Nope.. it doesn't exist, and it's very obviously a weird thing to be doing | 16:01 |
| niemeyer | TheMue: Similarly, it doesn't make sense to be doing watcher.Kill("Completely unrelated error") | 16:01 |
| TheMue | niemeyer: how do you handle it with tomb? tomb.Kill("Ouch") and later err := tomb.Wait() | 16:03 |
| niemeyer | TheMue: The tomb is an internal implementation of the watcher | 16:05 |
| niemeyer | implementation detail, that is | 16:05 |
| niemeyer | TheMue: There's nothing about the tomb in the watcher's interface.. we can drop it and create the same API without it | 16:05 |
| TheMue | niemeyer: yes, and i'm using this behavior, but only from a pipelined goroutines perspective | 16:06 |
| niemeyer | TheMue: There's no such thing as "pipeline goroutines".. We have a watcher, with an API | 16:06 |
| niemeyer | TheMue: The fact there's a goroutine inside it is irrelevant | 16:06 |
| niemeyer | TheMue: We could have 10 goroutines there, or none | 16:07 |
| TheMue | niemeyer: no, because the goroutine can't directly return an error | 16:07 |
| niemeyer | TheMue: All of that is merely an implementation detail we use to offer the actual API that is documented | 16:07 |
| niemeyer | TheMue: Kill isn't part of the API, and it makes no sense to have it there.. how would we document it? | 16:07 |
| niemeyer | TheMue: // Kill stops the watcher with the given error so that you can call Stop and get the error you provided | 16:08 |
| niemeyer | TheMue: ? | 16:08 |
| TheMue | niemeyer: it is documented, see the prpopose | 16:08 |
| niemeyer | It's a bad abstraction | 16:08 |
| TheMue | niemeyer: so how would you tell the watcher to end its work and pass the error that led to the error to the user of the watch? | 16:09 |
| niemeyer | TheMue: Again, fkill.. it makes no sense | 16:09 |
| niemeyer | TheMue: The watcher knows about errors in the watch | 16:09 |
| niemeyer | TheMue: It's not supposed to be a conduit for errors happening in logic it has no idea about | 16:09 |
| TheMue | niemeyer: the watcher (the type Watcher) doesn't know about the error | 16:10 |
| niemeyer | TheMue: Exactly, and it shouldn't know about *any* external errors | 16:10 |
| niemeyer | t | 16:10 |
| TheMue | niemeyer: the error happens in a goroutine listening to the change | 16:11 |
| niemeyer | TheMue: Exactly | 16:11 |
| niemeyer | TheMue: That's the place to have the error.. | 16:11 |
| TheMue | niemeyer: and i have to signal the watcher to stop working | 16:11 |
| niemeyer | TheMue: That's called Stop | 16:11 |
| TheMue | niemeyer: and the Stop() of the service config watcher then has to check if the own stored error is nil and return the value of Watcher.Stop() and otherwise the own error | 16:13 |
| niemeyer | TheMue: It has to do whatever it takes to handle its own problems.. calling foo.PleaseSaveMyError(err) so that foo.HaveYouFailed() when foo is an independent and self-contained abstraction is not nice | 16:14 |
| TheMue | niemeyer: we use tomb as a tool inside the type Watcher, and we use the Watcher inside the special watcher. so why two different kinds of abstraction, why shouldn't care Watcher for its own error management? | 16:16 |
| niemeyer | TheMue: I've been explaining it for a while.. Watcher *is* handling its *own* error management | 16:17 |
| niemeyer | TheMue: What you want is for it to handle somebody else's error management | 16:17 |
| niemeyer | TheMue: and that's not nice | 16:17 |
| niemeyer | TheMue: state/watcher shouldn't be a conduit for somebody else's errors | 16:18 |
| TheMue | niemeyer: tomb is doing the error management of Watcher | 16:18 |
| niemeyer | TheMue: Yes, that's its only purpose in life | 16:18 |
| niemeyer | TheMue: Just like mutexes to mutexing, conditional vars to conditional varing, etc.. those are not exposed either. | 16:18 |
| TheMue | niemeyer: the proposed way Watcher provides a clean API from a users perspective | 16:19 |
| niemeyer | TheMue: It's not a clean API, for the reasons I've been explaining repeatedly | 16:20 |
| niemeyer | TheMue: There's no fkill.. let's not have one in the watcher.. | 16:21 |
| TheMue | niemeyer: i'll change it | 16:22 |
| TheMue | niemeyer: but i would like you to do the review first, so i can do the changes at once | 16:24 |
| niemeyer | TheMue: Sure, I'll have a more careful look | 16:24 |
| niemeyer | TheMue: I love "observices", btw.. we should totally have a place for that somewhere in Juju :-) | 16:43 |
| TheMue | niemeyer: ;) | 16:43 |
| TheMue | niemeyer: funny thought mixup into one word | 16:44 |
| TheMue | niemeyer: should register it as trademark | 16:44 |
| niemeyer | TheMue: +1 :) | 16:45 |
| niemeyer | TheMue: The overall branch looks very nice, btw | 17:11 |
| niemeyer | TheMue: I love how this is evolving | 17:11 |
| niemeyer | TheMue: You have a proper review | 17:11 |
| niemeyer | TheMue: I'll move on and fix the tomb stuff we discussed re. tomb.Dying | 17:14 |
| TheMue | niemeyer: fine, thx | 17:17 |
| niemeyer | TheMue: Please let me know if the review looks sensible to you | 17:17 |
| TheMue | niemeyer: yip, will do, first have to fetch my dinner | 17:17 |
| niemeyer | TheMue: Oh man, if it's late for you that can wait until tomororw | 17:18 |
| TheMue | niemeyer: it's ok, will at least take a look | 17:43 |
| niemeyer | TheMue: Do you want to have a quick look at the tomb changes before they go in? | 17:53 |
| niemeyer | TheMue: Or, would you mind? | 17:54 |
| TheMue | niemeyer: oh, with pleasure | 17:54 |
| niemeyer | TheMue: Cool, pushing | 17:54 |
| niemeyer | TheMue: https://codereview.appspot.com/5981052 | 17:57 |
| TheMue | *click* | 17:57 |
| niemeyer | TheMue: As you will have noticed, also renamed ErrStillRunning to ErrStillAlive, as it was off within the naming context | 17:59 |
| TheMue | niemeyer: Yip, seen. Makes sense. | 18:01 |
| niemeyer | TheMue: Have you had a chance to look? | 18:17 |
| TheMue | niemeyer: almost done | 18:18 |
| niemeyer | TheMue: Superb, thanks | 18:18 |
| TheMue | niemeyer: just wanted to get a better insight in tomb. | 18:21 |
| niemeyer | TheMue: That's great for sure | 18:21 |
| TheMue | niemeyer: review is in | 18:31 |
| TheMue | niemeyer: took a look at your review, most parts are ok (even if i still find adding adding an own tom to the ConfigWatcher increases complexity, not much, but it does). and i'll move the ConfigWatcher from service.go to state.go | 18:32 |
| niemeyer | TheMue: Cheers! | 18:36 |
| niemeyer | TheMue: I'll put the defer into that case where the function is a bit more involved | 18:42 |
| niemeyer | TheMue: The other cases don't look necessary | 18:42 |
| TheMue | niemeyer: i like using defer for a linear execution Open()/defer Close(), Dial()/defer Close() etc. Fire and forget, no nesting. | 18:44 |
| niemeyer | TheMue: Yeah, that's cool in many cases.. but when it's a short lived and simple function, the defer is merely overhead | 18:49 |
| TheMue | niemeyer: how much overhead? shouldn't be premature optimization | 18:51 |
| niemeyer | TheMue: This isn't premature optimization.. this is pointless deoptimization.. :) | 18:51 |
| niemeyer | TheMue: It's assignment.. | 18:51 |
| niemeyer | It's an assignment, that is | 18:52 |
| niemeyer | Lock; a = t.foo; Unlock | 18:52 |
| niemeyer | TheMue: What are you defering for? | 18:52 |
| niemeyer | TheMue: You're right about Kill, though.. it looks better | 18:53 |
| TheMue | niemeyer: keeping an idiom to establish it more and more so that the code is more maintainable, regardless if the method is a one-liner or more complex | 18:53 |
| niemeyer | TheMue: Lock; trivial; Unlock is a fine idiom too, looks nice, is clean, and happens to use less resources than a defer | 18:54 |
| niemeyer | TheMue: Again, you're right about Kill, though | 18:55 |
| TheMue | niemeyer: ok | 18:56 |
| niemeyer | TheMue: I've updated the code, in case you'd like to look or LGTM it | 18:57 |
| niemeyer | TheMue: Although, it's just your suggestion | 18:57 |
| TheMue | niemeyer: *click* again ;) | 18:57 |
| TheMue | niemeyer: lgtm | 18:59 |
| niemeyer | TheMue: Cheers! | 19:00 |
| niemeyer | TheMue: Thanks for the suggestion as well | 19:00 |
| TheMue | niemeyer: np, my pleasure | 19:01 |
| TheMue | niemeyer: i'm off for today, cu tomorrow | 19:18 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!