[01:17] jelmer, hey. is any of it making sense to you now? [01:17] this issue... [01:42] spiv, hi [01:44] nm === medberry is now known as med_out [04:41] * maxb catches up on a bit of ~bzr PPA update backlog [04:42] hi maxb! [04:42] morning [05:26] Hmm. Not sure if I'm doing something wrong, but I'm finding the process of doing something in bzr to model having copied the beta-ppa line of bzr packages into the main ppa to be somewhat awkward [05:26] hm [05:27] do you want to talk more about it? [05:27] Well, I have a way which works, it's just not pretty [05:27] cd ....../ppa/natty [05:27] bzr merge ../../beta-ppa/natty [05:28] bzr revert -r-1:../../beta-ppa/natty [05:28] bzr ci [05:28] cd ../maverick [05:28] bzr merge ../../beta-ppa/maverick [05:28] hm [05:28] bzr merge --force ../natty [05:28] bzr revert -r-1:../../beta-ppa/maverick [05:28] bzr ci [05:28] continue for lucid [05:29] so you want to make a merge, that actually replaces everything with the origin? [05:29] you could just pull, perhaps [05:29] It would be an --overwrite [05:30] right [05:30] but that's arguably the reality here [05:30] hm [05:31] it would be better if there was a revspec for "my pending merge tip" so you could revert to that, too [05:31] i think there's a bug asking for it [05:31] yes, yes it would [05:31] I may have even filed it. Or me-too-ed it [05:36] how on earth have I ended up with differing file-ids in the lucid ppa branch for various packaging bits?! [05:37] oh, no [05:37] just conflict files left behind [05:38] but I do now have conflicting tags between beta-ppa/natty and ppa/natty! [05:46] :/ [05:46] that could actually be coming out of the workflow you discuss? [05:47] since there are going to be effectively two different revisions for 2.4.1-blah [05:47] There should never be that [05:47] One of the conflicting tags is 'bzr-2.3.3' ! [05:48] the rest are packaging in nature [05:49] er, whoops [05:49] * maxb realizes the need to revert . not revert in the above workflow [05:52] right, or it will lose your pending merge [05:53] i assumed that was just a typo - otherwise the ci will fail [06:40] hi all ! [06:42] Morning vila [06:42] 2.4.1 building PPA [06:42] hay maxb ! [06:42] \o/ [06:42] thanks a ton ! [06:42] er, building in PPA [06:43] we have an issue with bzr-builddeb. Its tests seem to be hinting at a bug in bzrlib.tests [06:43] yeah, I figured ;) (I'm pretty good at deciphering tyops, lots of practice in producing them myself) [06:44] ha ha, tell me more [06:44] AttributeError: type object 'TestCaseWithMemoryTransport' has no attribute '_SAFETY_NET_PRISTINE_DIRSTATE' [06:45] riiiings a bell [06:45] what and where was it... [06:45] known and fixed issue anyway [06:46] my background daemon is whispering jelmer or Riddell [06:46] or.... you? [06:46] :-) [06:47] Aug 30 12:18:08 _SAFETY_NET_PRISTINE_DIRSTATE was introduced recently, let me check [06:47] fakeroot [06:47] bzr/2.4 r6024 looks like it might fix it [06:47] Sounds a bit like something not calling the super class's setUp? [06:48] hi spiv, vila [06:48] test isolation says my IRC logs, reading [06:48] hey poolie ! [06:48] i'll retry the bzr-builddeb builds once bzr 2.4.1 has built [06:49] maxb: lp:bzr/2.4 revno 6024 is: (jameinel) Bug #609187, [06:49] check that packaging import branches are up-to-date when accessing them. [06:49] (John A Meinel) here [06:49] Launchpad bug 609187 in bzr (Ubuntu) "users are not warned when branching ubuntu:foo (or lp:ubuntu/foo) and the package import of foo is out of date" [High,Fix released] https://launchpad.net/bugs/609187 [06:49] 6042 !! [06:50] pff, who said he can't figure out tyops ? Lier [06:50] maxb: yup, that's exactly the one [06:51] pff, who said he *can* figure out tyops ? Lier [07:25] morning all [07:28] hi jam [07:28] vila, so it looks like the changelog merge hook is working ok? [07:29] poolie: ha, ha, quizz question first: how long does it take to the package importer to queue/process all tracked packages ? [07:30] to check them all? [07:30] i'm not sure, good question [07:30] a couple of hours? [07:30] nothing to win, I was surprised by the answer myself and like to see what people feel it is [07:30] exactly my feeling, ~2 hours [07:30] vila: queue and check when there is nothing to do? [07:30] jam: not exactly, on average, how long between two attempts for the same package [07:31] jam: no cheating by looking at the logs, first thought ! [07:32] vila: I'm not quite sure what you're getting at. It should be trying to pull the data of what needs to be done next from LP [07:32] maybe you can just tell us? [07:32] Are you saying to retry a failed package? [07:33] ~36 hours [07:35] i.e. on average, we try to import a given package every 36 hours [07:35] far more than the ~2 hours gut feeling I had [07:36] hence why I wanted you to tell your gut feeling before I told you [07:36] ok [07:37] well, that could certainly be faster [07:37] where did you get that data? from sampling the logs? [07:37] yup [07:37] perhaps eventually it could look at a feed-like view from launchad [07:38] looking at the output of https://code.launchpad.net/~vila/udd/analyze-log-imports/+merge/74057 instead of tail -F progress_log gives a better feeling [07:38] holistic feeling that is [07:40] poolie: and from that and to come back to your original question: no evidence yet that dpkg-mergechanlogs broke something, but still a bit more time needed to know that it has been tried for all packages [07:41] i.e. tonight, I'll mark the bug as fixed with reasonable confidence and still ask for it to be re-opened if needed [07:41] but I'm already convinced it's ok [07:42] 2011-09-15 05:00:30,456 - __main__ - INFO - All packages requeued, start again [07:43] so roughly the next occurrence is expected today ~17h00 UTC [07:44] vila, so you're on bug 795321 now? [07:44] Launchpad bug 795321 in Ubuntu Distributed Development "udd importer should make tea while launchpad is down" [High,In progress] https://launchpad.net/bugs/795321 [07:44] jam that was very quick on the disconnect stuff [07:44] is there anything i can do on it? [07:44] poolie: yup, I have a crcuit breaker implemented and tested, I'm now trying to plug it into mass_import [07:44] nice [07:45] three events: attempt (to import), success, failure [07:45] one assumption is that when lp is down, no import can succeed [07:46] another is that failures are already classified, some of them are transient [07:46] I expect the transient ones to be easily linked to lp down [07:49] and the final one is that we have a way to say: this failure is a transient lp one from the backtrace and/or because it raises launchpadlib.HTTPError [07:50] ideally from the command line [07:51] bbiab [07:54] poolie: you can test the disconnect stuff if you like, it seems to work fine here. On Windows and Natty at least. [07:55] I think the next obvious step is to hook into that with a SIGUSR1/SIGHUP [07:55] and then get the client to gracefully reconnect [07:55] vila, one option for concurrency is to change to use tdb [07:55] i'm pretty sure that's multi-writer [07:55] and simple [07:55] and udd does very simple things with its db [07:56] either that or whichever nosql is fashionable today [07:56] poolie: well, you could just switch to postgres [07:56] Most nosql solutions do very poorly at low scale [07:56] for example, mongodb defaults to pre-allocating 5% of your disk space (in my case, 1GB) [07:57] awesome [07:57] i wasn't very serious about that actually [07:58] poolie: I didn't think you were. As for tdb, we could, but sqlite seems much more well tested. With the WAL work for sqlite 3.6 or so [07:58] the actual contention between readers and writers is tiny [07:58] the only issue is if you need multi writer [07:59] i think tdb is pretty reliable, though less commonly used [07:59] poolie: On the note of nosql -> http://howfuckedismydatabase.com/nosql [07:59] upgrading sqlite would be a smaller change [07:59] nigelb: :) [07:59] i like that [07:59] more to the point, http://howfuckedismydatabase.com/sqlite/ [07:59] heh [08:04] so would anyone (maxb?) agree or disagree with me in bug 831699 that to track success, it's probably cleanest just to add a success table? [08:04] Launchpad bug 831699 in Ubuntu Distributed Development "no report/log of successful packages" [High,Confirmed] https://launchpad.net/bugs/831699 [08:04] and/or refactor the 'failure' thing into an 'outcome' that can be either success or failure [08:04] poolie: right, not there yet (migrating from sqlite ;) [08:05] ? [08:05] making tea has issues with concurrency but not related to the db [08:06] . o O (Go decipher that joe random ;) [08:06] For the circuit breaker, the fact that lp can go down while several imports are running has two outcomes: [08:07] aloha [08:07] - you can see a failure *followed* by a success because the failure see lp down while the success still had work todo to finish the import that didn't require lp [08:08] so the success is a false positive as far as lp state is concerned [08:08] right [08:08] poolie: I think adding a success table makes sense. It gives you a place to say *when* it last succeeded, how long it took, whatever other stats you want to track [08:08] so there has to be some kind of trending [08:08] one swallow does not make a spring [08:09] poolie: I always burn my coats when I see a swallow.... [08:09] - you can see a failure related to lp but the classification is wrong (permanent instead of transient) which is also a false positive [08:10] poolie: right, so as far as the circuit breaker is concerned, there is little interest in waiting for more success as long as we keep trying on transient failures [08:11] vila: you could also just have it be a soft timeout that increments on failure, and decrements on success. [08:11] (not necessarily by the same amount) [08:11] another issue is that there is no fast and unambiguous way to decide if lp is up [08:11] also [08:11] so start a new package every 30s, if they are failing, make it 45, then 60, then... [08:12] jam: right, but the issue here is more about lag [08:12] I don't know how long a package will need to tell me lp is up [08:13] so I'm more inclined to say: we have a max_threads for mass_import, [08:13] could you look at the code i quote in bug 831699 [08:13] Launchpad bug 831699 in Ubuntu Distributed Development "no report/log of successful packages" [High,In progress] https://launchpad.net/bugs/831699 [08:13] it seems wrong but i might be missing the point [08:15] what is wrong ? [08:15] OLD_FAILURES ? [08:15] why does it chec kfor a failures entry, and only if that exists delete the old job [08:15] I think the JOB table is populated only if you had a failure [08:16] or if it's a new package [08:17] err [08:17] won't work for new packages, forget that [08:17] no, i think there's always a job created when it starts, and it's marked closed higher up in this function [08:18] so the intention certainly seems to be that they're kept around, but inactiev [08:18] also, why delete it if it failed on the previous attempt [08:18] poolie: "I wonder what the logic is behind deleting the job if there was previously a failure record." [08:18] I think it is deleted on any success, indicating that it doesn't need to be run again [08:18] (job completed) [08:18] so that it doesn't appear on the web page while it's queued ? [08:19] ah, nm, I see your point [08:19] poolie: I think it is just faulty logic. It seems like it checks for row, because it wanted to use it, or something like that. [08:19] mass_import starts by queuing the job table and only when it's empty does it look at the package table [08:20] i think it's a mismatch [08:20] well, i have to go out now [08:20] also note that the 'delete from %s' doesn't follow the rest of the SQL refactoring that pulls strings out into constants, etc. [08:20] maybe james will reply [08:20] poolie: have a good night [08:20] exactly [08:20] i colud annotate it [08:20] . o O (my empire for a test framework there) [08:20] poolie: g'night [08:21] nothing obvious there [08:21] ok, we'll see [08:21] i'll track successes on monday [08:21] cheerio [09:37] in add_hook e.g. self.add_hook('transform_fallback_location', "Called when a stacked branch is activating its fallback " etc is the description ever shown to the user? [09:49] are the strings in check.py check() self.progress.update() user visible? [09:58] Riddell: "bzr help hooks" ? [09:58] I'm not sure about the check.py stuff [09:58] but if it is "progress", then yes, most likely user-visible [09:59] vila: did you ever get a chance to re-review the patch I put up? I think I ironed out the kinks [10:00] jam: not yet [10:00] I think poolie's only comment was that I should probably be checking "errors" [10:00] yup, I agree with that [10:00] I was hoping to have a way to actually trigger that, so that I know the code works [10:01] I can list a few themes I want to mention without formally reviewing if you wish [10:01] or do that later more formally [10:10] vila: feedback is feedback if you have time to list it out [10:11] medium has a disconnect method, why do you need to implement a _close() one ? [10:13] the config stuff could be revisited once bug #491196 is fixed, in the mean time, what you did is good, [10:13] Launchpad bug 491196 in Bazaar "want a way to set configuration options from the command line" [High,Confirmed] https://launchpad.net/bugs/491196 [10:14] I wouldn't require plugins to support the new timeout parameter (but I'm not sure you had a choice there) [10:14] vila: client side has .disconnect. not server side [10:14] I can call it "disconnect" if you like [10:15] vila: I don't see a way to pass the timeout parameter optionally, other than what I did [10:15] try/TypeError [10:16] jam: but is there a way to not *force* the plugins to accept it (i.e. could they just ignore it to start with and implement it later) [10:16] i.e. is loggerhead *required* to take it into account *today* [10:16] vila: I did that, try/pass_5_arguments/except TypeError/pass 4 arguments [10:16] with a deprecation warning inbetween [10:17] vila: I did mention that loggerhead works *today* with a warning. [10:17] which is suppressed in release builds [10:17] vila: I know it isn't obvious, but too-many-arguments is a TypeError [10:17] in python [10:18] jam: oh, I may have looked at on old version then, I don't remember seeing this part [10:18] s/on/an/ [10:18] ok, good then [10:18] vila: possible, though I think I implemented except TypeError when I implemented the command line. [10:19] vila: client side has .disconnect. not server side [10:19] but test server side has shutdown_client (client from the server side pov) [10:20] there may be a way for the tests to use a native disconnect() if available [10:20] vila: only the test server, not this implementation [10:20] vila: what do you mean by 'native disconnect'? [10:20] SmartServerStreamMedium does not have the concept of disconnecting the client yet [10:21] you added shutdown_client only in the Test implementations [10:21] native as in supported by the server side of the client [10:21] yes, to limit the scope at the time and at least for the SmartTCPServer because spiv said he didn't care [10:21] vila: SmartServerStreamMedium *is* the server side of the client. I really feel like I'm missing something here. [10:22] vila: I'm happy to rename ._close() to .disconnect() and to change shutdown_client() to call .disconnect() [10:22] the test infrastructure didn't try to use it because that was the only existing one [10:22] though it doesn't quite match because of *what* self.clients tracks [10:22] ha [10:22] it doesn't track the medium/handlers, it tracks the actual socket connections. [10:22] that may be where the mismatch is, [10:23] I feel like you're adding stuff that is existing but may be that's because the dots are not connected [10:23] and a fallout of doing that may explain the weird behaviors you're seeing [10:24] and I'm still uncomfortable with the select loop as I have a feeling that it's needed because these dots are not connected [10:25] vila: fundamentally the test infrastructure is poking at a thread's internal state from another thread. [10:25] I don't think that is a 'stable' situation. [10:25] We do it because parts of the thread are blocking [10:25] on top of that, if the SmartTCPServer ends up needing to track its connections, it sounds like the this code should be shared [10:26] vila: well, *today* SmartTCPServer does *not* track its connections. They are set to Daemon and forgotten. [10:26] the alternative was to raise an exception in the client thread from the server thread but back in the days this wasn't easy to do and still be compatible with 2.4, 2.5 and 2.6 [10:27] vila: this is like the thread.interrupt_main()? [10:27] vila: In my last comments I noted something [10:27] which is that *doesn't* interrupt socket.accept() [10:27] jam: which is why you encounter issues with "interpreter shutdown" and need to to keep references to sys.stderr and the line [10:27] it waits for it to timeout/return first [10:27] like [10:27] vila: so "interrupting" the client thread doesn't actually do such a thing [10:27] because it is blocked in a C lib [10:27] so you *still* need to do a loop [10:28] which loop ? the select one ? [10:28] vila: In that case, a loop around socket.accept() [10:28] (SmartTCPServer.serve) [10:28] it wanted a loop already [10:28] because it wants to support multiple connections [10:28] There is a test case in blackbox [10:28] that calls "thread.interrupt_main()" [10:28] interrupt_main sounds like a thread interrupting the *main* thread which can receive signals [10:28] and it doesn't actually interrupt until socket.accept() returns [10:28] you can see my notes on it [10:29] I'm talking about raising an exception *from* the main thread in another one [10:29] but if you have "socket.settimeout(1)" it takes 1s for the test to shutdown [10:29] evil, don't do that [10:29] vila: sure, *my* point is that raising an exception doesn't actually interrupt select.select() or socket.accept() or socket.recv() etc. [10:29] vila: we already have that [10:29] I "fixed" it with an optional "change the timeout parameter. [10:29] where ? [10:29] bzr selftest -s bb.test_serve [10:29] I forget the exact test [10:30] blackbox.test_serve.TestCmdServeChrooting.test_serve_tcp [10:30] And I've seen that happen with one of the other tests [10:30] I think it is a race condition with whether it gets blocked in the socket.accept() before it gets a chance to raise the exception. [10:33] vila: so our SmartTCPServer already does a 1 second timeout loop in serve [10:33] I've long suspected race conditions but 1) I stopped encountering them when adding the necessary sync points, 2) I'm not convinced anymore that *python* itself have some [10:34] vila: sync points? [10:34] jam: on the listening socket, irrelevant, this one is ok [10:34] vila: except it makes the test take 1s to shut-down [10:34] which I override down to 0.1s [10:34] but that's not a race [10:34] vila: there *is* a race in another test, where it sometimes waits an extra second to shutdown after calling thread.interrupt_main() [10:35] it isn't strictly a 'race' [10:35] as in, it always gives the same results [10:35] but how long it takes varies [10:35] because the 'main' thread is blocked [10:35] waiting on socket.accept(). I would expect the same thing for select.select() [10:35] because the 'thread.interrupt_main()' *doesn't* use signals [10:35] so if the call is in a C function [10:36] it is blocked from python seeing the 'you need to raise an exception' call. [10:36] vila: hence, you need a loop, to avoid blocking forever [10:36] vila: for example, if I wrote the _wait_for_timeout code to do select.select(..., timeout=300). I *think* you could not ^C the python process. [10:37] You technically *could*, but it may not actually trigger until the 300s times out [10:37] I've certainly seen stuff like ^C get blocked because we are in a C function. [10:38] interrupt_thread is not what I had in mind, it may shares some common parts but the one I remember was allowing raising a specific exception not KeyboardInterrupt [10:38] http://docs.python.org/library/thread.html?highlight=thread.interrupt_main#thread.interrupt_main [10:38] vila: so we sort of got off on a tangent. My specific point is that you need the loop, regardless of testing-specific interactions, because you aren't guaranteed that ^C will do what you want [10:38] says" Threads interact strangely with interrupts: the KeyboardInterrupt exception will be received by an arbitrary thread. (When the signal module is available, interrupts always go to the main thread.) [10:39] vila: you're looking at a different function than what you linked [10:39] jam: my point is: you shouldn't need the loop and we don't know *why* [10:39] ? [10:40] vila: http://paste.ubuntu.com/690681/ [10:40] you mentioned interrupt_main, I don't remember the link of the alternative [10:40] follow your own link [10:40] it doesn't say what you said [10:40] vila: ah, way down at the end? [10:46] vila: I can confirm that it is a Windows thing I'm seeing. [10:46] Specifically: socket.recv(1) is blocking on windows. [10:46] such that ^C doesn't interrupt it [10:46] and you have to kill the process by other means [10:46] on Linux, I get KeyboardInterrupt reliably [10:46] on Windows, once I finally send some data [10:47] then I see KeyboardInterrupt [10:47] I don't understand what you're talking about, interrupt)main ? [10:47] vila: so there are 2 things [10:47] 1) socket.recv() blocks ^C until it returns on Windows (not on linux) [10:48] 2) socket.accept() is known to block thread.interrupt_main() from raising KeyboardInterrupt until socket.accept() returns (on all platforms) [10:48] as in, it queues up a KeyboardInterrupt *to be raised when socket.accept returns* [10:48] forget interrupt_main, not all servers run in the main thread (most don't) [10:49] right, hence the need for the *test* which runs in the main thread, to act on the socket in the server context so that either the blocking call is unblocked or it raises an exception [10:49] vila: going further, select.select() [on Windows] blocks ^C until it returns [10:49] that's the whole idea of shutdown_client [10:50] jam: even when you specify the error parameter ? [10:50] vila: thread.interrupt_main() doesn't matter if it is main thread or not, it is still blocked until socket.accept() returns. [10:50] right, so, let's forget about it [10:50] vila: i'll try that, but given that KeyboardInterupt is raised the moment select.select() returns [10:51] vila: select.select([c], [], [c], 10) doesn't respond to ^C until I either wait the 10s or write a byte to the client socket [10:51] oh, right, TestBzrServeBase, now I remember that one, special case, probably the only one where the server runs in the main thread [10:51] or close the client socket ? [10:52] from the server end, not the client end [10:52] vila: my experience with testing, was that server-end *does not return* until the timeout if I close the socket in another thread. [10:52] I'll try again, though. [10:53] vila: *my* point, is that because select.select() blocks stuff like ^C until timeout [10:53] we should use a shorter timeout, and loop [10:53] and if we need a loop anyway [10:53] then we can not worry about it in the test suite. [10:53] dunno for select but I can assure you that it works for read() (for the client threads)) [10:54] so either the select is interrupted or the read is interrupted and as long as you catch the right exceptions you shouldn't need the loop around select [10:55] vila: there is no read(), do you mean recv() ? [10:55] yeah, recv, sorry [10:55] vila: and you mean thread.interrupt_main() or you mean ^C [10:55] ? [10:56] I mean whatever way is used to close the connection [10:56] vila: I'm setting up a test case for that on Windows. I'll let you know. [10:57] forget about interrupt_main, it's a hack and not a good example (useful shortcut for TestBzrServeBase though) [10:58] but TestBzrServeBase is about checking hook execution IIRC not really about how the server is interrupted [10:59] vila: ok, I can say that... testing a very simple test case threading.Thread(target = (sleep1, c.close()).start(); select([c], [], [], 10) returns before 10s saying that you can recv from the socket without blocking. [10:59] so not relevant for our current discussion because we *cannot* use it [10:59] on Windows [10:59] vila: I can also say, this was not borne out in the test suite [10:59] where sometimes it hangs forever [10:59] well, until timeout [10:59] borne out ? [10:59] vila: confirmed by [10:59] similar results with [10:59] oh, I can believe that [11:00] hangs are a pain to debug, therefore extremely hard to diagnose [11:00] http://answers.yahoo.com/question/index?qid=20100702014132AAcKdX8 [11:01] jam: thanks ! Keep them coming ! (And always fell free to fix my broken english) [11:01] vila: so. select.select() will block ^C on windows until timeout, and seems unreliable that we detect the socket getting closed underneath us (according to the test suite). [11:01] As such, it seems that a loop is reasonable. [11:01] ... [11:02] jam: before I fixed the test suite leaks, we *were* using a similar loop [11:02] with timeouts to make matte worse [11:02] vila: if only because you can't ^C the process until timeout, I don't think we should have a 300s timeout. [11:04] you lost me there, I thought the scope of the bug was lp, why should we pay for busy loop there ? [11:05] vila: the scope of the bug is "we'd like to disconnect clients that are idle for too long", a 1s sleep loop isn't a lot of wakeups, though certainly you could reduce it if you wanted. [11:06] vila: I feel pretty strongly that we *need* a loop on windows [11:06] I can have "if sys.platform == 'win32': loop" [11:06] That seems far worse than just having a loop, which handles all the current issues, even if there may be other issues in the future. [11:09] does selec([],[], [xxx]) block ? [11:09] does select([],[], [xxx]) block ? [11:10] vila: it blocks ^C until it returns, yes. [11:10] vila: are you saying with no timeout? [11:11] or are you saying with the socket passed in the errors field [11:11] the later [11:11] (and note, there is no *error* with ^C) [11:11] vila: *on Windows*, select.select() blocks until the C function underneath it returns, and then stuff like exception handling and signal processing occur [11:11] before the python function returns [11:12] jam: wow, and recv(), listen() etc all behave this way on windows ? [11:12]