| -opendevstatus- NOTICE: The maintenance of the review.opendev.org Gerrit service is now complete and service has been restored. Please alert us in #opendev if you have any issues. Thank you | 03:30 | |
| *** dpawlik0 is now known as dpawlik | 05:10 | |
| *** mgoddard- is now known as mgoddard | 06:04 | |
| *** dpawlik5 is now known as dpawlik | 06:50 | |
| *** hashar is now known as Guest1365 | 07:02 | |
| *** hashar_ is now known as hashar | 07:02 | |
| *** dpawlik7 is now known as dpawlik | 07:06 | |
| *** dpawlik3 is now known as dpawlik | 08:30 | |
| *** dpawlik4 is now known as dpawlik | 08:44 | |
| *** rpittau|afk is now known as rpittau | 09:31 | |
| opendevreview | Andy Ladjadj proposed zuul/zuul master: [DNM] use packedrefs for all refs https://review.opendev.org/c/zuul/zuul/+/801276 | 09:55 |
|---|---|---|
| opendevreview | Andy Ladjadj proposed zuul/zuul master: [DNM] use packedrefs for all refs https://review.opendev.org/c/zuul/zuul/+/801276 | 09:57 |
| opendevreview | Andy Ladjadj proposed zuul/nodepool master: [driver][aws] add a cache on retrieve image id https://review.opendev.org/c/zuul/nodepool/+/801288 | 11:14 |
| opendevreview | Andy Ladjadj proposed zuul/nodepool master: [driver][aws] add a cache on retrieve image id https://review.opendev.org/c/zuul/nodepool/+/801288 | 11:50 |
| opendevreview | Paul Belanger proposed zuul/nodepool master: First ensure ssh connection is valid before scanning keys https://review.opendev.org/c/zuul/nodepool/+/801189 | 12:59 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: Document RETRY build status https://review.opendev.org/c/zuul/zuul/+/801303 | 13:10 |
| *** sshnaidm|afk is now known as sshnaidm | 13:10 | |
| clarkb | corvus: I seem to recall mention of fixing a static node nodepool test? is test_static_parallel_increase the one that was failing? it hit pabelanger[m]'s change above if so. Do we have a fix up for that yet? | 14:45 |
| corvus | clarkb: that doesn't ring a bell | 14:48 |
| clarkb | https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_a28/801189/2/check/nodepool-tox-py36/a28686a/testr_results.html has the traceback. It looks like a zk issue trying to unlock a noderequest without holding the noderequest | 14:49 |
| clarkb | I'll recheck pabelangers change and see if it is persistent | 14:50 |
| clarkb | and then maybe run that test in a loop locally since that worked well the last time we had an issue | 14:51 |
| tristanC | is it possible to use the rebase merge mode in github (request from https://storyboard.openstack.org/#!/story/2009062 ) ? | 15:57 |
| clarkb | tristanC: zuul had support for it at one time with gerrit (not sure if it got ripped out). We found it created weird interactions that were not easily addressed. I don't recall the specifics though. | 15:59 |
| clarkb | Might be worthwhile digging into the history there (it is probably in the zuul git log somewhere?) and evaluating if those issues are still issues and whether or not they can be addressed for other hosting systems like github | 16:00 |
| clarkb | One problem was specific to gerrit and that was that it rewrote the commits (github may do the same?) | 16:01 |
| clarkb | this meant that even in a fast forward situation you never got the same commit hash out again | 16:01 |
| tristanC | clarkb: thank you, that is my recollection too | 16:01 |
| clarkb | "The rebase and merge behavior on GitHub deviates slightly from git rebase. Rebase and merge on GitHub will always update the committer information and create new commit SHAs, whereas git rebase outside of GitHub does not change the committer information when the rebase happens on top of an ancestor commit." | 16:04 |
| clarkb | https://docs.github.com/en/github/administering-a-repository/configuring-pull-request-merges/about-merge-methods-on-github#rebasing-and-merging-your-commits | 16:04 |
| clarkb | it seems that github has this problem too. Whether or not that is still a problem for zuul I don't know | 16:04 |
| clarkb | tristanC: did you want to respond to the story with some of that info? I can in a bit if you don't | 16:06 |
| corvus | here's a puzzle -- i'm looking into zk connection losses on opendev. check out the following log entries: https://paste.opendev.org/show/807564/ | 16:07 |
| corvus | the first traceback makes perfect sense to me. | 16:07 |
| corvus | the second traceback is very confusing -- how did we jump from the deferred exception rais in kazoo/handlers/utils.py to the gerrit connection election? | 16:08 |
| corvus | (line 35/36 of that paste) | 16:08 |
| corvus | 35 should have been the end... is there some data corruption happening inside of kazoo? | 16:09 |
| clarkb | corvus: is it possible the log messages ended up out of order and there is another "Traceback (most recent call last):" out of sequence somewhere? | 16:10 |
| clarkb | taht would mean we have 3 exceptions instead of 2 | 16:10 |
| clarkb | or at least 3 I guess | 16:10 |
| corvus | clarkb: i don't see any in the recent history above that point. below that is another disconnection and traceback; i think that's a separate event. | 16:11 |
| corvus | also, the individual chunks in that make sense | 16:12 |
| clarkb | in that case it certainly seems like call stacks are weird. THough I haven't dug into the specific code to be more confident of that | 16:12 |
| corvus | i've checked 4 disconnects so far, and they all have the same pattern in at least one TB. | 16:20 |
| tristanC | clarkb: i don't mind copying your answer, though it's not urgent and i'd rather let you do it if you don't mind | 16:24 |
| clarkb | tristanC: ok I'll get to that in a bit | 16:25 |
| tristanC | corvus: isn't that a property of AsyncResult? | 16:27 |
| opendevreview | Andy Ladjadj proposed zuul/zuul master: [DNM] use packedrefs for all refs https://review.opendev.org/c/zuul/zuul/+/801276 | 16:27 |
| tristanC | (that the exception can comes from another code path) | 16:27 |
| *** marios is now known as marios|out | 16:29 | |
| pabelanger[m] | clarkb: do have info about the failing non-voting nodepool test? is that what you were looking into | 16:29 |
| clarkb | pabelanger[m]: no I think that is expected due to the zk ssl work? the failure in the py36 unittests was what I asked about earlier | 16:30 |
| clarkb | pabelanger[m]: I don't think that failure is related to your chagne as it seems related to zk trying to unlock a nodrequest and failing | 16:30 |
| tobiash[m] | corvus: we've also experienced zk connection losses after updating on saturday. We had to rollback | 16:30 |
| pabelanger[m] | okay, great. thanks for looking | 16:31 |
| tobiash[m] | corvus: if you check the logs for 'zookeeper connection' you may see it toggles between suspended and connected | 16:31 |
| pabelanger[m] | if I could get a few reviews on https://review.opendev.org/c/zuul/nodepool/+/801189, it would be helpful for us on zuul.a.c. Even if we could schedule a release for it too | 16:31 |
| corvus | tristanC: the exception attached to asyncresult should just include a stack trace from the call stack within the kazoo client; i don't understand why the call stack there would include the leader election | 16:31 |
| tobiash[m] | corvus: swest also discovered a merged but yet unrelease fix on the kazoo connection level: https://github.com/python-zk/kazoo/pull/633 | 16:32 |
| tristanC | since the traceback contains two get_children_async call, could it be that the handler is mixing the two here: https://github.com/python-zk/kazoo/blob/master/kazoo/client.py#L1235 | 16:47 |
| corvus | tristanC: i wonder -- it's certainly suspicious that both tracebacks in my paste relate to get_children on the same znode | 16:50 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: Document RETRY build status https://review.opendev.org/c/zuul/zuul/+/801303 | 16:54 |
| clarkb | tristanC: https://storyboard.openstack.org/#!/story/2009062 has been updated with some of my notes from above | 17:25 |
| tobiash[m] | clarkb, tristanC commented on ^ | 17:32 |
| *** rpittau is now known as rpittau|afk | 17:38 | |
| tobiash[m] | pabelanger: lgtm | 17:42 |
| opendevreview | Chao Zhang proposed zuul/zuul-jobs master: Update commits since tag calculation https://review.opendev.org/c/zuul/zuul-jobs/+/801370 | 18:03 |
| opendevreview | Chao Zhang proposed zuul/zuul-jobs master: Update commits since tag calculation https://review.opendev.org/c/zuul/zuul-jobs/+/801370 | 18:04 |
| opendevreview | Merged zuul/zuul master: Document RETRY build status https://review.opendev.org/c/zuul/zuul/+/801303 | 18:26 |
| opendevreview | Merged zuul/nodepool master: First ensure ssh connection is valid before scanning keys https://review.opendev.org/c/zuul/nodepool/+/801189 | 18:49 |
| opendevreview | Chao Zhang proposed zuul/zuul-jobs master: Update commits since tag calculation https://review.opendev.org/c/zuul/zuul-jobs/+/801370 | 19:59 |
| opendevreview | Chao Zhang proposed zuul/zuul-jobs master: Update commits since tag calculation https://review.opendev.org/c/zuul/zuul-jobs/+/801370 | 20:00 |
| opendevreview | James E. Blair proposed zuul/zuul master: Split ZK event queues by type https://review.opendev.org/c/zuul/zuul/+/801387 | 21:37 |
| corvus | clarkb: i think that's something you suggested on an earlier review? | 21:38 |
| clarkb | corvus: yup, as it simplifies the idea of what a queue is :) | 21:38 |
| clarkb | I'll give it a review | 21:38 |
| corvus | i'm about to make some changes to the event queues so that they shard large events (there's a possibility that's what caused the problems tobiash et al saw over the weekend), so i figured that would be a good first step before further changing the queue structure | 21:39 |
| corvus | clarkb: i think it's failing my local testing -- so if you do look at it at this point, maybe just review it for the overall concept :) | 21:41 |
| clarkb | noted | 21:41 |
| clarkb | corvus: the overall shape of the change lgtm. One thought is how will migrating to that look for those of us running the old system? will we orphan those events? probably fine if so | 21:49 |
| clarkb | or maybe they are ephemeral already | 21:49 |
| corvus | clarkb: i believe we'll orphan them | 21:58 |
| corvus | coming up soon on my list is the "delete everything from zk" cli utility; we can use that to clean up the cruft | 21:58 |
| clarkb | corvus: https://review.opendev.org/c/zuul/zuul/+/799127 is also related to some feedback I left on a change I think. Any idea what is up with testing on that one? | 22:10 |
| corvus | okay, found the issue; was missing an ensure_path; it's not a substantial change. will upload new ps when local tests finish. | 22:10 |
| corvus | clarkb: no, i haven't looked into that yet. | 22:11 |
| opendevreview | James E. Blair proposed zuul/zuul master: Split ZK event queues by type https://review.opendev.org/c/zuul/zuul/+/801387 | 22:14 |
| corvus | that should be gtg now | 22:14 |
| corvus | now on to sharding | 22:14 |
| clarkb | corvus: you didn't remove the tenant specific string. | 22:14 |
| clarkb | not a big deal but you called it out so pointing it out | 22:15 |
| opendevreview | James E. Blair proposed zuul/zuul master: Split ZK event queues by type https://review.opendev.org/c/zuul/zuul/+/801387 | 22:15 |
| corvus | clarkb: derp thx | 22:15 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 23:44 |
| corvus | okay that's step 2. i think 1+2 are the minimum needed to address potential issues with large events and results. i think we should also do one more change which is to utilize that new system to forward events with the data in-place; that should further reduce zk traffic and churn. | 23:46 |
| corvus | well, that's a lot harder than it sounds (because some of the forwarding methods use quite a bit of the event data, so it needs to be read anyway). might still be able to avoid a write, but it's not a slam-dunk. i may stop here for now. | 23:55 |
| corvus | swest, tobiash: please take a look at https://review.opendev.org/801387 and https://review.opendev.org/801402 and let me know what you think | 23:56 |
| opendevreview | Paul Belanger proposed zuul/nodepool master: Add release note for previous commit https://review.opendev.org/c/zuul/nodepool/+/801403 | 23:57 |
| pabelanger[m] | clarkb: tobiash corvus: I should have included a release note in my change, to allow for a nodepool release. | 23:58 |
| pabelanger[m] | ^ | 23:58 |
| clarkb | pabelanger[m]: one msall thing but if we have you here should be a quick update (left it on the chnage) | 23:59 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!