fwereade | davecheney, heyhey | 08:24 |
---|---|---|
rogpeppe | fwereade, davecheney: morning' | 08:25 |
fwereade | and rogpeppe, also heyhey | 08:25 |
fwereade | and TheMue :) | 08:25 |
TheMue | fwereade: morning, and who else? ;) | 08:26 |
fwereade | TheMue, dave and rogpeppe | 08:26 |
TheMue | fwereade: ah | 08:26 |
rogpeppe | i have only just opened up the computer, to be fair :-) | 08:26 |
TheMue | davecheney: hello and rogpeppe: heya | 08:26 |
rogpeppe | TheMue: yo@ | 08:27 |
rogpeppe | ! | 08:27 |
* TheMue too | 08:27 | |
rogpeppe | my fingers still aren't working too well | 08:27 |
davecheney | hello | 08:28 |
fwereade | davecheney, and possibly rogpeppe, I was meaning to ask | 08:32 |
fwereade | davecheney, rogpeppe: is it just a tools problem that's blocking non-precise deploys? | 08:32 |
rogpeppe | fwereade: yes | 08:32 |
rogpeppe | fwereade: AFAIK | 08:32 |
davecheney | fwereade: i have a proposal for that | 08:32 |
fwereade | davecheney, rogpeppe: in that case ISTM that davecheney's approach is sane | 08:33 |
rogpeppe | fwereade: we just need to compile mongodb for quantal | 08:33 |
rogpeppe | fwereade: at least, that was the blocker i saw | 08:33 |
davecheney | rogpeppe: i'm only considering deploying from quantal -> LTS | 08:33 |
fwereade | rogpeppe, ah, ok, makes sense | 08:33 |
davecheney | we can consider the other permutations another day | 08:33 |
rogpeppe | davecheney: we want to be able to do LTS -> quantal too | 08:33 |
davecheney | but making a quantal/raring mongo will enable that | 08:33 |
davecheney | rogpeppe: we *might* | 08:33 |
davecheney | i haven't seen a request for that yet | 08:34 |
rogpeppe | davecheney: so that we can run the builddb charm for quantal from precise | 08:34 |
davecheney | rogpeppe: what is the story with mongo | 08:34 |
fwereade | davecheney, we want to be able to deploy to and from anything basically | 08:34 |
davecheney | fwereade: sure, i agree, but i'm only focusing on things we *need* this week | 08:34 |
davecheney | wants can come later :) | 08:34 |
rogpeppe | davecheney: we just need to build a quantal version | 08:34 |
davecheney | rogpeppe: i thought we were screwed on licencing | 08:34 |
fwereade | davecheney, that's fine indeed :) | 08:34 |
rogpeppe | davecheney: i haven't heard that - what's the story? | 08:35 |
davecheney | rogpeppe: mark saied we aren't allowed to distribute that binary | 08:35 |
rogpeppe | davecheney: the mongo binary? | 08:35 |
davecheney | ja | 08:35 |
rogpeppe | davecheney: well we're fucked then | 08:35 |
davecheney | yes | 08:36 |
davecheney | is mark working this week ? | 08:36 |
davecheney | rogpeppe: ideally if we could get a 2.2 backport into the archive | 08:36 |
davecheney | that would solve our problem | 08:36 |
rogpeppe | davecheney: how would that help? | 08:36 |
rogpeppe | davecheney: and how come we're allowed to distribute the binary via apt-get ? | 08:37 |
davecheney | rogpeppe: as I understand it the current mongo in precise has TLS enabled | 08:37 |
davecheney | is that correct | 08:37 |
davecheney | ? | 08:37 |
rogpeppe | davecheney: i dunno. i don't think so, but i'll check | 08:38 |
davecheney | rogpeppe: if so, the licencing issue becomes someone elses' problem | 08:38 |
davecheney | we just ride on the archives' coat tails | 08:38 |
rogpeppe | davecheney: no the default version does not have tls enabled | 08:39 |
davecheney | rogpeppe: right, back to being screwed then | 08:39 |
rogpeppe | davecheney: i thought AGPL allowed distribution in binary form | 08:40 |
davecheney | openssl is not AGPL :( | 08:40 |
davecheney | and by linking statically to it | 08:40 |
davecheney | we polute the binary | 08:40 |
davecheney | but, we might get lucky as the source of the program that builds the binary is available | 08:41 |
* davecheney IANAL | 08:41 | |
rogpeppe | davecheney: we weren't planning to link to openssl, were we? | 08:42 |
rogpeppe | davecheney: ah, of course | 08:42 |
rogpeppe | davecheney: mongo itself | 08:42 |
rogpeppe | doh! | 08:42 |
davecheney | welcome to open sores licencing shitfighting | 08:42 |
rogpeppe | davecheney: openssl is so shit internally too. not worth fighting for. | 08:44 |
rogpeppe | davecheney: the license doesn't seem too onerous actually: http://www.openssl.org/source/license.html | 08:45 |
rogpeppe | davecheney: (either of them) | 08:45 |
rogpeppe | davecheney: " | 08:45 |
rogpeppe | Actually both licenses are BSD-style | 08:45 |
rogpeppe | Open Source licenses. | 08:45 |
rogpeppe | " | 08:45 |
davecheney | rogpeppe: I need mark to respond | 08:46 |
davecheney | but for the moment lets assume we're in the clear | 08:46 |
rogpeppe | davecheney: it looks like we should be. those licenses don't seem to disallow anything other than redistributing without the license | 08:47 |
rogpeppe | davecheney: oh, here's the issue: http://people.gnome.org/~markmc/openssl-and-the-gpl.html | 08:49 |
davecheney | right, openssl is bsd 3 clause | 08:50 |
davecheney | am i correct ? | 08:50 |
rogpeppe | davecheney: yeah | 08:50 |
rogpeppe | davecheney: so it imposes some restrictions (you must distribute acknowledgements) that GPL doesn't allow you | 08:51 |
rogpeppe | davecheney: what a mess | 08:51 |
davecheney | sounds like the Apache Harmony vs the Java TCK nonsense | 08:53 |
rogpeppe | davecheney: i don't know about that | 08:57 |
* rogpeppe googles | 08:57 | |
davecheney | rogpeppe: http://en.wikipedia.org/wiki/Apache_Harmony#Difficulties_to_obtain_a_TCK_license_from_Sun | 08:58 |
davecheney | almost identical story to the one above | 08:58 |
davecheney | to do X, we need Y, but using Y implys we can't be X | 08:59 |
davecheney | error: Failed to load data for project "juju-core": Get https://api.launchpad.net/devel/juju-core: EOF | 09:01 |
* davecheney shakes fist at LP | 09:01 | |
rogpeppe | davecheney: if either mongodb or openssl were reasonable pieces of code, it should not be hard to do a GPL-compatible replacement for the way that mongo uses it | 09:03 |
rogpeppe | davecheney: sadly both are convoluted to hell | 09:03 |
davecheney | so i have heard | 09:05 |
rogpeppe | davecheney: i spent a significant proportion of yesterday delving deeply into the openssl code | 09:05 |
rogpeppe | davecheney: so i could find a particular constant | 09:05 |
rogpeppe | davecheney: to implement x509.Encrypt | 09:06 |
rogpeppe | oops | 09:06 |
rogpeppe | davecheney: to implement x509.EncryptPEMBlock | 09:06 |
rogpeppe | davecheney: because mongo does not support reading unencrypted PEM blocks | 09:06 |
rogpeppe | davecheney: it epitomises everything that's wrong with C-as-it-has-turned-out | 09:07 |
* davecheney pats rogpeppe on the shoulder | 09:08 | |
davecheney | i bet it's a rats nest of macros and #ifdefs | 09:08 |
rogpeppe | davecheney: yup | 09:08 |
rogpeppe | davecheney: and unnecessary layers of abstraction | 09:09 |
rogpeppe | davecheney: so this issue means that none of the Go source code is GPL-compatible AFAICS | 09:12 |
rogpeppe | davecheney: and i always thought the BSD license was the less restrictive one | 09:13 |
davecheney | rogpeppe: it is a modified 2 arg, right ? | 09:13 |
rogpeppe | davecheney: well, i guess it is, but just an incompatible set of restrictions | 09:13 |
rogpeppe | davecheney: "Redistributions in binary form must reproduce ..." | 09:14 |
* davecheney checks | 09:15 | |
davecheney | i don't know enough about this stuff | 09:15 |
davecheney | i'm not sure i want to ,either | 09:15 |
rogpeppe | GPL should allow components to add copyright notice distributions | 09:15 |
rogpeppe | i'm sure that this incompatibility is against its original aims | 09:15 |
davecheney | rogpeppe: i think go is ok in this respect | 09:16 |
davecheney | the wording in the 2nd clause in | 09:17 |
davecheney | http://golang.org/LICENSE | 09:17 |
davecheney | differs from the complaint in http://people.gnome.org/~markmc/openssl-and-the-gpl.html | 09:17 |
rogpeppe | " | 09:18 |
rogpeppe | You may not impose any further | 09:18 |
rogpeppe | restrictions on the recipients' exercise of the rights granted herein. | 09:18 |
rogpeppe | " | 09:18 |
davecheney | oh for fucks sake | 09:18 |
rogpeppe | the 2nd clause in the go license seems like it does that | 09:18 |
rogpeppe | yeah | 09:18 |
davecheney | maybe this is an ordering issue | 09:18 |
davecheney | the order in which you stack licences would allow for some interesting permutations | 09:19 |
rogpeppe | davecheney: so you can't distribute a Go program that links against a GPL program, but you can distribute a Go program as GPL | 09:21 |
rogpeppe | davecheney: that kinda makes sense | 09:21 |
davecheney | sounds like witchcraft | 09:22 |
rogpeppe | davecheney: i see now how 10gen make money | 09:22 |
davecheney | you can't sell wisky with water in it, but you can pour wisky over ice | 09:22 |
davecheney | or something | 09:22 |
davecheney | rogpeppe: for 10gen i imagine it is more about indemity | 09:22 |
davecheney | or providing indemnity to their customers | 09:22 |
davecheney | which is a massive issue for the more regulated companies in our industry | 09:23 |
rogpeppe | davecheney: i doubt it - why else would they sell the OpenSSL version separately? | 09:23 |
rogpeppe | davecheney: well, i'm sure indemnity is part of it too | 09:23 |
davecheney | you are probably right, heinoius money grabbing | 09:23 |
rogpeppe | davecheney: actually, this just might be a good thing | 09:28 |
davecheney | mmm | 09:30 |
rogpeppe | davecheney: actually.... i think we're all ok | 09:30 |
rogpeppe | davecheney: AGPL != GPL | 09:30 |
rogpeppe | davecheney: see http://www.gnu.org/licenses/agpl-3.0.html section 7 | 09:30 |
rogpeppe | davecheney: "you may [...] supplement the terms of this License with terms: [...] requiring preservation of specified reasonable legal notices or author attributions in that material or in the Appropriate Legal Notices displayed by works containing it; " | 09:31 |
davecheney | the source of Juju is AGPL, i dunno if that applies to a file we reference in binary form | 09:32 |
rogpeppe | davecheney: mongo license is AGPL | 09:32 |
davecheney | orly | 09:32 |
rogpeppe | davecheney: which i'd always thought of as more restrictive than GPL, funnily | 09:33 |
davecheney | it's more free, which depends on your POV | 09:33 |
rogpeppe | davecheney: anyway, by my reading of it, we *can* distribute mongodb binaries :-) | 09:34 |
rogpeppe | davecheney: IANALBASOTI | 09:35 |
davecheney | sounds good enough for now | 09:35 |
davecheney | yes, everyone in the chan who is a lawyer, raise your hand | 09:35 |
davecheney | rogpeppe: so, where does that leave us | 09:36 |
davecheney | mongo == good | 09:36 |
rogpeppe | davecheney: yeah | 09:36 |
rogpeppe | davecheney: AGPL == good | 09:36 |
davecheney | but you're getting screwed over by certs | 09:36 |
rogpeppe | davecheney: that's fine, go needs EncryptPEMBlock anyway | 09:36 |
rogpeppe | davecheney: although it is annoying | 09:37 |
Aram | hello. | 11:00 |
TheMue | Aram: hi | 11:01 |
=== mcclurmc is now known as mcclurmc_away | ||
=== dimitern is now known as dimitern_lunch | ||
hazmat | rogpeppe, we do have lawyers avail | 12:53 |
hazmat | mramm, was looking into it afaicr | 12:54 |
rogpeppe | hazmat: i just wanted to assure myself that all was not lost :-) | 12:54 |
mramm | hazmat: rogpeppe: well we can move to encrypted channels in the rest API | 12:55 |
rogpeppe | mramm: what about the channels from the API servers to the mongodb servers? | 12:55 |
mramm | the thing there is that you could snoop data if you can snoop the network | 12:55 |
mramm | right | 12:55 |
=== dimitern_lunch is now known as dimitern | ||
mramm | you wouldn't have control, just access to data, which is not great, but probably not catastrophic | 12:56 |
rogpeppe | mramm: if you can change network packets, you have control | 12:56 |
mramm | true | 12:56 |
rogpeppe | mramm: from my extremely crude reading of the AGPL, i don't *think* there should be a problem with us distributing the mongodb/openssl binaries. | 12:57 |
rogpeppe | mramm: but as hazmat says, we have lawyers. i'd like to know what they think. | 12:57 |
mramm | yes | 12:58 |
mramm | I think we might be able to get away with distributing under a modified AGPL that allows advertising | 12:59 |
mramm | but we might need to talk 10gen into doing the same | 12:59 |
mramm | and elliot at 10gen seems to think he has a solution to the problem coming | 12:59 |
mramm | but I'm not in the loop on what that is | 13:00 |
=== mcclurmc_away is now known as mcclurmc | ||
mramm | and we can also just setup VPN of some sort to handle transport level security | 13:02 |
mramm | so it's definitely true that "all is not lost" | 13:03 |
mramm | we will be able to route around the problem if we need to | 13:03 |
fwereade | Aram, ping | 13:04 |
Aram | yes | 13:04 |
Aram | fwereade: ^ | 13:04 |
fwereade | Aram, I'm thinking about subordinates | 13:04 |
Aram | what about them. | 13:05 |
fwereade | Aram, and ISTM that what I need is basically precisely a machiner, but watching a slightly different set of units | 13:05 |
fwereade | Aram, but I wanted you input on the current MachinePrincipalUnitsWatcher | 13:06 |
fwereade | Aram, and whether or not you anticipate changes to API or semantics at any point | 13:06 |
Aram | fwereade: so what set of units you want to watch? | 13:06 |
fwereade | Aram, the subordinates of one specific unit | 13:07 |
Aram | I see. | 13:07 |
Aram | well. | 13:07 |
Aram | hmm. | 13:07 |
Aram | there will be changes in the implementation that will make this easier, hopefully this week. | 13:07 |
Aram | but in semantics or API, I don't think so. | 13:07 |
fwereade | Aram, so it remains just Added/Removed with all other changes swallowed? | 13:08 |
Aram | (actually the MachinePrincipalUnitsWatcher and the MachineUnitWatcher use a diferent API in my latest branches than in trunk). | 13:09 |
Aram | personally I would change it to return more events. | 13:09 |
Aram | but I don't think niemeyer would approve. | 13:10 |
Aram | for the time being I suspect it's a safe bet that this behavior won't change. | 13:10 |
fwereade | Aram, always a fine line to walk ;) | 13:10 |
Aram | although it has disadvantages IMO. | 13:10 |
fwereade | Aram, I'll go with the current behaviour for now then, see where it takes me; cheers | 13:11 |
Aram | fwereade: it kind of bugs me that each collection watcher starts N individual watchers, but in some client of the collection watcher, like the firewaller or whatever, you start N individual watchers yourself anyway. | 13:12 |
fwereade | Aram, yeah, I feel that it's undergone somewhat forced/accelerated evolution and it needs to settle a bit | 13:13 |
Aram | fwereade: I' | 13:13 |
Aram | fwereade: I'd either make collection watchers return everything (no filtering) OR make them report only new objects as they become alive and no other event. | 13:14 |
Aram | fwereade: I did an experiment yesterday making the machine units watcher return everything with no filtering. the "problem" was that you got an event for the principal when you added a subordinate, because the principal is also modified in the process, but maybe that's ok. | 13:19 |
fwereade | Aram, yeah, I don't see that as a major problem myself | 13:20 |
Aram | fwereade: well, one issue might be that it essentially returns implementation details, right now the principal is modified in the process of adding a subordinate, but in the future it need not be. | 13:21 |
fwereade | Aram, I guess it all comes back down to agreement on what changes it is guaranteed for and which it isn't | 13:22 |
Aram | TheMue: have any idea why a firewaller will pick a close port event, but it will not pick the same close port event if the firewaller was stopped, and then restarted? | 14:18 |
Aram | I have a mental idea of what kind of race is going on, but it's somewhat hard for me to debug it, probably because of the unfamiliarity of the code. | 14:19 |
Aram | TheMue_: have any idea why a firewaller will pick a close port event, but it will not pick the same close port event if the firewaller was stopped, and then restarted? | 14:19 |
Aram | I have a mental idea of what kind of race is going on, but it's somewhat hard for me to debug it, probably because of the unfamiliarity of the code. | 14:20 |
TheMue_ | Aaarg, disconnected just after the answer. | 14:20 |
TheMue_ | Aram: I will take a deeper look. | 14:20 |
TheMue_ | Aram: The last part that has been changed has been the adding of the global mode. | 14:21 |
=== TheMue_ is now known as TheMue | ||
TheMue | Aram: Here the initial open ports are retrieved not by the watchers but by traversing the state. | 14:22 |
Aram | hmm | 14:22 |
Aram | TheMue: basically this works, but if you uncomment those lines it will fail: http://paste.ubuntu.com/1339904/ | 14:23 |
TheMue | Aram: Where exactly does it fail? Line? | 14:25 |
Aram | TheMue: http://paste.ubuntu.com/1339921/ | 14:26 |
Aram | TheMue: you say that initial ports are retrieved from the environment, not from the watchers, then how can the firewaller determine if a close port on a unit should have any effect globally, since it doesn't know which units might use that port? | 14:32 |
Aram | TheMue: from what you are telling me, the failure seems expected, so how did it work before? | 14:32 |
TheMue | Aram: Just going through the code. | 14:32 |
TheMue | Aram: It worked. | 14:32 |
TheMue | Aram: The initial ports only in global mode are retrieved from the env. | 14:33 |
Aram | yeah | 14:33 |
TheMue | Aram: But the watchers still work. | 14:33 |
Aram | of course. | 14:33 |
TheMue | Aram: And this is collated with the already known global ports. | 14:33 |
Aram | but when you get a close port event from a unit, how do you know if you should close the port globally? | 14:33 |
TheMue | Aram: One moment, will tell you the line number. | 14:34 |
TheMue | Aram: Line 228 and the function flushGlobalPorts() | 14:35 |
TheMue | Aram: Here is also the fw.globalPortRef[port] to cound the usage | 14:36 |
rogpeppe | lunch | 14:36 |
Aram | TheMue: who initializes fw.globalPortRef? | 14:36 |
TheMue | Aram: It's created in initGlobalMode() and the counters will be increased by flushGlobalPorts() (based on the watchers). | 14:38 |
TheMue | Aram: If they would already be counted in initGlobalMode() they would be counted twice. | 14:39 |
fwereade | TheMue, on a related note, if you have a mo: how do we handle units which are removed with opened ports? ISTM that they will keep those ports open, but I am also unfamiliar with the code | 14:39 |
TheMue | fwereade: In global mode ports will be closed after the final unit using this port. | 14:40 |
TheMue | fwereade: Otherwise directly. | 14:40 |
Aram | fwereade: I think it's fine, removal of a unit event makes fw call flush machine which calls flushGLobalPorts and does the refcound decrement | 14:41 |
TheMue | fwereade: If you change it to dying the firewaller gets notified by the watcher. | 14:42 |
fwereade | Aram, ah! forgetUnit | 14:42 |
fwereade | TheMue, thanks | 14:42 |
TheMue | fwereade: yw | 14:42 |
fwereade | TheMue, wait, when what becomes Dying? | 14:42 |
fwereade | TheMue, do we do something when the unit becomes Dying? | 14:42 |
Aram | TheMue: take a look at this, maybe you see an obvious flaw with it: https://codereview.appspot.com/6820112 | 14:43 |
TheMue | fwereade: I would have to look more exactly. | 14:43 |
Aram | I'm not sure that the failure I see is due to my code, but rather to just enabling a race, but maybe my code sucks. | 14:43 |
TheMue | fwereade: Will look at Arams review before. | 14:44 |
fwereade | TheMue, I just didn't unerstand what you meant by: | 14:44 |
fwereade | <TheMue> fwereade: If you change it to dying... | 14:44 |
TheMue | fwereade: Please later, I'm not working concurrent | 14:44 |
fwereade | TheMue, ok, I would appreciate a clarification when you have a mo :) | 14:45 |
TheMue | fwereade: Yep, will do afterwards. | 14:45 |
fwereade | TheMue, cheers | 14:45 |
TheMue | fwereade: Have to look the exact flow in the code | 14:46 |
fwereade | TheMue, I just want to know what you meant by "it" in "setit to dying" above... I don't think you need to look at the code... | 14:46 |
TheMue | fwereade: Has just been a quick rememberance. The important part is that the watchers notify the fw and the fw combines these changes to get aware which ports it has to open/close. | 14:48 |
TheMue | Aram: Hmm, so far it looks good. The loops over the removed and added units are almost the same, only that you now have a list with names and a list with units. But I'll again check the part above. | 15:07 |
TheMue | Aram: The retrieval of the lifecycle state is now somewhat strange. Shall it be changed this way everywhere? I liked the fact that the watchers emit changes according to the state. | 15:08 |
Aram | TheMue: could you be more specific or rephrased that? I don't understand what you mean. | 15:08 |
TheMue | Aram: Sure, so far the change itself contained Added and Removed. Now you get all changes and have to check the state on you own. That looks strange. | 15:11 |
Aram | TheMue: well that's how the new style watchers are now. | 15:11 |
TheMue | Aram: I liked the old way, or a return of Alive, Dying, Dead to be more close to the states. | 15:11 |
Aram | eventually, all old style watcher will be like this. | 15:11 |
Aram | we took the decision of doing it this way at Lisbon. | 15:12 |
TheMue | Aram: OK, then I think it's well discussed. Forget my question. | 15:12 |
fwereade | TheMue, I'm looking at the firewaller again | 15:14 |
TheMue | fwereade: Feel free | 15:14 |
fwereade | TheMue, in the watch loops for unitData and serviceData, you defer watcher.Stop() | 15:14 |
fwereade | TheMue, is that intentional | 15:15 |
fwereade | ? | 15:15 |
fwereade | TheMue, ie, doing so swallows errors, but I can see a fuzzy justification for why we might want to | 15:15 |
Aram | TheMue: so you don't see anything egregious about my code? | 15:16 |
fwereade | TheMue, but I need help clarifying what it is and whetherit exists ;) | 15:16 |
TheMue | Aram: So far not, it looks fine to me. | 15:16 |
Aram | hmm. | 15:16 |
TheMue | fwereade: Sorry, here I have to go into history when they wen in. Since the earliest steps the fw has had several changes. | 15:17 |
fwereade | TheMue, sure, this is pretty tightly focused though | 15:17 |
Aram | I think that there's a race in the firwaller, and my code helped bring it to the surface, but the actual fault is in the firewaller, not my code. | 15:17 |
fwereade | TheMue, why do we ignore errors in sd.watcher and ud.watcher? | 15:17 |
Aram | TheMue: btw, if I change the MachineUnitsWatcher to report everything, not only lifecycle changes, this test passes. | 15:18 |
TheMue | fwereade: Please one after the other. | 15:18 |
fwereade | TheMue, I asked the same question in two different ways, I thought | 15:19 |
TheMue | And btw, lunchtime. Today later, daughter came just back. | 15:19 |
fwereade | TheMue, ok, I'll see what happens if I get stricter :0 | 15:20 |
fwereade | rogpeppe, Aram, TheMue: I have to go early today, but before I do... I just proposed https://codereview.appspot.com/6811091 ... | 15:40 |
fwereade | which makes service and unit watchers just return ids instead of entities | 15:41 |
fwereade | but I now want to go a step further, and cause machine/unit/service watchers to just send `struct {}{}` | 15:41 |
Aram | fwereade: heh, I already did that as well. | 15:41 |
fwereade | rationale is that whoever started the watch should damn well know what enitity they mean | 15:42 |
Aram | I might have even proposed it about two weeks ago. | 15:42 |
Aram | but gustavo didn't like a prereq | 15:42 |
fwereade | Aram, crap, sorry, I never saw that | 15:42 |
Aram | so he didn't even look over it | 15:42 |
fwereade | Aram, ah hell | 15:42 |
Aram | and never went in | 15:42 |
Aram | anyway | 15:42 |
rogpeppe | fwereade: i totally agree, but i've said that a few times to gustavo and had no agreement | 15:56 |
TheMue | back | 15:56 |
fwereade | rogpeppe, hum, ok -- the issue is that it lets me dodge the damn-stupid-in-hindsight difference in type between unit and machine | 15:57 |
fwereade | sorry, unit and machine *ids* | 15:58 |
rogpeppe | fwereade: i agree | 15:58 |
Aram | fwereade: rogpeppe: one benefit of returning id is that a collection watcher can literally be constructed from N individual watchers muzed together on the same channel. | 15:58 |
Aram | s/muzed/muxed/ | 15:58 |
Aram | not that we do that, just saying. | 15:58 |
rogpeppe | Aram: that would be an issue if you passed a channel into a watcher | 15:58 |
rogpeppe | Aram: as it is, it's not AFAICS | 15:58 |
rogpeppe | fwereade: if you can demonstrate a nice use case for struct{}, i think it would probably be fine | 15:59 |
Aram | TheMue: I think I know what the race is. | 15:59 |
* TheMue listens | 15:59 | |
Aram | TheMue: individual machine and unit watchers are started as soon as possible, rather than waiting for the fw to finish reading the environment. | 15:59 |
fwereade | rogpeppe, I think the putative deployer makes a nice one | 15:59 |
rogpeppe | fwereade: agreed | 15:59 |
fwereade | rogpeppe, but maybe for now I'll ponder less-controversial prereqs for doing so -- I should at least fix up MachinePrincipalUnitsWatcher | 16:00 |
Aram | rogpeppe: yeah, we don't currently make use of that feature, and the benefit of having watchers return the same type outweighs the potential benefit of muxing watchers in the future. | 16:00 |
fwereade | Aram, +1 | 16:01 |
Aram | fwereade: I have a fix for that as well | 16:01 |
Aram | I'm just blocked on debugging this stupid thing | 16:01 |
Aram | that's why is not up for review again | 16:01 |
fwereade | Aram, ah ok! sorry, would you link me your branch for MPUW so I can see what I'll need to work with? | 16:01 |
TheMue | Aram: IMHO not. loop() starts with initGlobalMode() before entering the watchers. But let me have a deeper look. | 16:03 |
Aram | fwereade: well, this one: https://codereview.appspot.com/6718052/ but I'll have to work on it some more to use more of the machine units watcher that has changed in the meantime and the prereq is bad as well. | 16:04 |
TheMue | Aram: Ah, no, WatchMachines() starts earlier, indeed. | 16:05 |
Aram | TheMue: yes, you're right, the environment is read before, but the behavior suggests otherwise. I've put debug prints inside the unit watcher loop, and globalPortRef is not good | 16:05 |
Aram | basically empty | 16:05 |
Aram | meh | 16:07 |
Aram | for whatever reason the unit loop doesn't run enought times. | 16:07 |
TheMue | Aram: Maybe the separation in added and removed should be done inside the unitd to avoid receiving from the watcher the next time. The slices of added and removed can then be sent to the main loop as before. | 16:12 |
Aram | TheMue: what do you mean? | 16:13 |
TheMue | Aram: I'm jist thinking loud, but I'm thinking it will show no effect. :| | 16:14 |
TheMue | Aram: It's the new new loop where you initially check if a units is added or removed before you later iterate over those slices to process the units like before. | 16:14 |
Aram | TheMue: you want to do it in a single loop? | 16:15 |
Aram | why do you think it should make a difference? | 16:15 |
TheMue | Aram: No. During this "pre-processing" the unitd can receive the next change, maybe here's the race. So a solution can be to move the pre-processing into the unitd and then send the separated slices to the main loop. | 16:16 |
TheMue | Aram: But that's only guessing. | 16:17 |
Aram | TheMue: I don't know whay you mean by 'unitd'? | 16:17 |
Aram | s/whay/what/ | 16:17 |
Aram | what is unitd? | 16:17 |
TheMue | unitData | 16:18 |
TheMue | They are managed as fw.unitds[] | 16:18 |
Aram | but unitDatas are created inside the for _, unit := range added { by newUnitData, aren't they? | 16:20 |
Aram | so they don't even exist in the preprocessing step\ | 16:20 |
TheMue | Aram: Yes, those both loops (removed, added) can stay in the fw.loop(), as before. Only the first loop that's new now.. | 16:21 |
Aram | TheMue: I don't understand what difference that makes, but maybe I am missing something basic. | 16:22 |
Aram | oh | 16:22 |
Aram | hmm | 16:22 |
Aram | no | 16:22 |
Aram | I don't see it :). | 16:22 |
Aram | I mean that function is called synchronously, as if the code was pasted there. | 16:23 |
TheMue | Aram: Yes, you're right. *sigh* | 16:23 |
Aram | TheMue: on the other hand you might be right about the race, when that function is executing, more events could be queued for the watcher, and the fw would call that function again with the same units. | 16:24 |
Aram | not sure what difference that makes though. | 16:24 |
TheMue | Aram: Could be worth a test. | 16:24 |
Aram | TheMue: inlining that and doing it all in a single loop, without a preprocessing step, made no difference. | 16:36 |
TheMue | Aram: Sh... | 16:36 |
Aram | TheMue: I'm out of ideas to try. | 16:42 |
Aram | TheMue: the fact that it works if you don't restart the watcher suggests there's something wrong with watcher initialization. | 16:47 |
TheMue | Aram: Don't know. It worked so far. So the change seems to effect a different behavior inside the fw. | 16:52 |
Aram | TheMue: what about var ports []state.Port inside func (ud *unitData) watchLoop() {? | 16:59 |
Aram | this wil start fresh, from nil when watcher is restarted | 16:59 |
Aram | but that function compares the new ports with old ports each run | 16:59 |
Aram | so it will compare with nil when a watcher is started | 17:00 |
Aram | so it won't ever close ports | 17:00 |
Aram | yeah | 17:00 |
Aram | that's it. | 17:00 |
Aram | it compares nil to 80, and can't realize it needs to close 8080 | 17:00 |
Aram | TheMue: does that make sense? | 17:01 |
TheMue | Aram: Sounds good, I'm only wondering why it worked before. | 17:02 |
TheMue | Aram: I'll go into the history there tomorrow to get a feeling. | 17:04 |
Aram | TheMue: because the watcher delivered units as they were at that point, so you'd get one even with 80, 8080 and one with 80, but now you have to load the unit manually, and when you load it it's already only 80 | 17:04 |
* TheMue leaves now, I've got a cold and almost had no sleep last night. | 17:04 | |
TheMue | Aram: Ah, that sounds reasonable. | 17:05 |
Aram | TheMue: have a nice evening | 17:05 |
TheMue | Aram: Thx, cu tomorrow | 17:05 |
TheMue | Aram: And have a nice evening too | 17:05 |
=== mcclurmc is now known as mcclurmc_away | ||
=== mcclurmc_away is now known as mcclurmc | ||
=== rog is now known as Guest70098 | ||
=== mcclurmc is now known as mcclurmc_away | ||
=== mcclurmc_away is now known as mcclurmc | ||
=== mcclurmc is now known as mcclurmc_away |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!