| opendevreview | Paul Belanger proposed zuul/nodepool master: Add release note for previous commit https://review.opendev.org/c/zuul/nodepool/+/801403 | 00:03 |
|---|---|---|
| pabelanger[m] | done, thanks! | 00:04 |
| clarkb | +2 | 00:05 |
| *** swest[m] is now known as swest | 05:20 | |
| *** rpittau|afk is now known as rpittau | 07:07 | |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Log result payload size of merger jobs https://review.opendev.org/c/zuul/zuul/+/801422 | 07:54 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Copy tenants dictionary before modification https://review.opendev.org/c/zuul/zuul/+/801439 | 10:02 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Sort tenant list by name https://review.opendev.org/c/zuul/zuul/+/801442 | 10:17 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Use a temp ZK config cache for tenant validation https://review.opendev.org/c/zuul/zuul/+/800800 | 10:26 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Use a temp ZK config cache for tenant validation https://review.opendev.org/c/zuul/zuul/+/800800 | 11:45 |
| opendevreview | Felix Edel proposed zuul/zuul master: Make reporting asynchronous https://review.opendev.org/c/zuul/zuul/+/691253 | 12:19 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 13:27 |
| corvus | swest: ^ that implements your suggestion | 13:28 |
| opendevreview | Felix Edel proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/787686 | 13:30 |
| opendevreview | Merged zuul/nodepool master: Add release note for previous commit https://review.opendev.org/c/zuul/nodepool/+/801403 | 14:30 |
| *** dviroel is now known as dviroel|lunch | 14:54 | |
| clarkb | corvus: I'll take a look at https://review.opendev.org/c/zuul/zuul/+/801402/ after breakfast too | 15:02 |
| pabelanger[m] | clarkb: corvus: thanks for approving. Could I request a 4.2.2 release of nodepool? | 15:09 |
| corvus | zuul-maint: does this look good for nodepool? commit 20358459769a4fad29cd0a7173547f8f2e37eb32 (HEAD -> master, tag: 4.2.2, origin/master, refs/changes/03/801403/2) | 15:33 |
| clarkb | 20358459769a4fad29cd0a7173547f8f2e37eb32 as 4.2.2 lgtm. pabelanger[m]'s change is the only one after 4.2.1 | 15:35 |
| fungi | well, almost. it's two changes but one adds the release note for that | 15:43 |
| fungi | regardless, i agree that looks like a fine 4.2.2 candidate | 15:43 |
| clarkb | corvus: left some questions for you on https://review.opendev.org/c/zuul/zuul/+/801402/ | 15:47 |
| corvus | pushed 4.2.2 | 15:48 |
| opendevreview | Matthieu Huin proposed zuul/zuul-client master: API client: Enable CORS compatibility https://review.opendev.org/c/zuul/zuul-client/+/801507 | 15:50 |
| corvus | clarkb: replied; i think maybe i didn't communicate effectively what was being sharded (only the data, not the metadata) | 15:54 |
| clarkb | corvus: aha yes I was conflating sequences and shards (I think they have a similar underlying mechansim but the behaviors are different here) | 15:57 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: [WIP] [api][cors] Add CORS configuration https://review.opendev.org/c/zuul/zuul/+/767691 | 15:58 |
| corvus | yep, that'll do it | 15:58 |
| clarkb | corvus: for the order of operations thing in the last comment my main concern is what happens if the event entry is removed successfully then we fail to delete the data entries due to some error that isn't a NoNodeError. Also if for some reason get a nonodeerror from the event delete we may never delete the data ? | 15:59 |
| clarkb | corvus: thinking the more verbose option is more robust to failures that may occur | 15:59 |
| mhu1 | Apologies for taking 767691 over but I think it's going to be needed by the auth patch chain | 15:59 |
| corvus | clarkb: agreed -- i'll update that in a bit; we could also probably do that in a transaction.... not sure which would be better... | 16:00 |
| *** dviroel|lunch is now known as dviroel | 16:00 | |
| *** marios is now known as marios|out | 16:22 | |
| clarkb | I'm working up a response for y2kenny's question on the mailing list and from what I read in the zuul change ansible connection vars are not settable in any job. | 16:45 |
| *** rpittau is now known as rpittau|afk | 16:45 | |
| fungi | yes, i think the idea was they would be set by nodepool when creating the nodes | 16:52 |
| fungi | ahh, you mentioned that in your reply | 16:53 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 16:58 |
| corvus | clarkb: ^ not that verbose as it turns out, thanks to suppress :) | 16:58 |
| clarkb | that reads really well too. I have learned a thing about python | 17:01 |
| corvus | swest put it in the original form of that method, i'm happy i can put it back :) | 17:02 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: [api][cors] Add CORS configuration https://review.opendev.org/c/zuul/zuul/+/767691 | 17:45 |
| opendevreview | James E. Blair proposed zuul/zuul master: Use the nodeset build parameter instead of hosts/groups https://review.opendev.org/c/zuul/zuul/+/799127 | 18:04 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Use kopf operator framework https://review.opendev.org/c/zuul/zuul-operator/+/785039 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Bump API version to v1alpha2 https://review.opendev.org/c/zuul/zuul-operator/+/785047 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Run a git server in k8s for functional tests https://review.opendev.org/c/zuul/zuul-operator/+/785738 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Move ingress to functional test https://review.opendev.org/c/zuul/zuul-operator/+/785757 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Support externally managed Zookeeper and DB https://review.opendev.org/c/zuul/zuul-operator/+/785273 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Pass through extra scheduler config options https://review.opendev.org/c/zuul/zuul-operator/+/785277 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add merger support https://review.opendev.org/c/zuul/zuul-operator/+/785278 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add keystore password support https://review.opendev.org/c/zuul/zuul-operator/+/790182 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Support imagePrefix and versions https://review.opendev.org/c/zuul/zuul-operator/+/785279 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Support fingergw https://review.opendev.org/c/zuul/zuul-operator/+/785300 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add docs https://review.opendev.org/c/zuul/zuul-operator/+/785083 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Support zuul-preview https://review.opendev.org/c/zuul/zuul-operator/+/785760 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add support for zuul-registry https://review.opendev.org/c/zuul/zuul-operator/+/785761 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Remove extra 2 minute wait from tests https://review.opendev.org/c/zuul/zuul-operator/+/785762 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add allowUnsafeConfig database setting https://review.opendev.org/c/zuul/zuul-operator/+/785764 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Pass through environment to scheduler, web and launcher https://review.opendev.org/c/zuul/zuul-operator/+/785988 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Allow terminationGracePeriodSeconds to be configurable https://review.opendev.org/c/zuul/zuul-operator/+/785989 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Flake8 cleanups https://review.opendev.org/c/zuul/zuul-operator/+/786349 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix error with multiple nodepool providers https://review.opendev.org/c/zuul/zuul-operator/+/799917 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add instructions and tools for running tests with kind https://review.opendev.org/c/zuul/zuul-operator/+/785763 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command https://review.opendev.org/c/zuul/zuul-operator/+/800263 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger https://review.opendev.org/c/zuul/zuul-operator/+/800264 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic https://review.opendev.org/c/zuul/zuul-operator/+/800786 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Document externalConfig https://review.opendev.org/c/zuul/zuul-operator/+/800791 | 20:28 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix config update detection https://review.opendev.org/c/zuul/zuul-operator/+/800991 | 20:28 |
| corvus | ianw, clarkb: patch bombs are much faster with the new gerrit. kudos. :) | 20:28 |
| corvus | i squashed the test fix into the first change, so that series should be gtg now | 20:29 |
| ianw | haha well conversely i don't know what element-desktop is doing to display notifications but that stack just gave me about 30+ seconds of 100% cpu :) | 20:30 |
| corvus | ianw: heh -- maybe something to do with url previews? | 20:31 |
| clarkb | corvus: I'm glad to hear it is faster :) | 20:35 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: [api][cors] Add CORS configuration https://review.opendev.org/c/zuul/zuul/+/767691 | 20:44 |
| clarkb | corvus: https://review.opendev.org/c/zuul/zuul/+/799127/3 is still failing testing. That was next on my list of zuul changes to review. Do you think it is worthwhile despite the failure? | 20:46 |
| clarkb | it only failed one of the unittests so I assume it is close (new races?) | 20:46 |
| corvus | clarkb: i ran a full test suite locally, so i think we got it. i think it's worth reviewing | 20:46 |
| clarkb | corvus: got it | 20:46 |
| corvus | clarkb: https://review.opendev.org/801402 hit timeouts. i ran tests locally and it adds a cumulative 478s test time for me (and about a 5.6% increase in wall-clock time) | 20:55 |
| corvus | i feel like that's worth looking into, though i'm unsure if we can do much about it. | 20:56 |
| clarkb | corvus: huh I wouldn't have expected that to cause much slowness | 20:56 |
| clarkb | I guess it is making more znodes which is more io and synchronization | 20:56 |
| corvus | yeah, and maybe a little bit of cpu time with the compression? | 20:56 |
| corvus | ideas off the top of my head: forward events by reference (though we still need to read some of the event data in, so that's not a slam dunk). and maybe only create the side data node if it's large enough | 20:57 |
| corvus | i didn't want to do that last thing right away because it would mean we would almost never execute that code path in tests, but would frequently do so in prod :/ | 20:57 |
| clarkb | we'd have to artificially inflate a test or two's data? | 20:58 |
| corvus | yep. | 20:58 |
| corvus | those two ideas are slightly contrary -- | 20:58 |
| corvus | if we forward data by reference, that only helps if we're actually storing data in a separate node | 20:59 |
| clarkb | if we forward by reference we'll still read the data at some point though? | 21:01 |
| corvus | yeah, but only one round trip instead of two | 21:01 |
| corvus | they're also complimentary -- we can do both things | 21:02 |
| corvus | it's just that they relate to each other | 21:02 |
| corvus | i think the right choice probably depends on whether we think we would really need to shart frequently in production. based on my knowledge, i think it might be best to avoid sharding unless necessary. i think most events are small. it's just occasionally you get a giant blob from github or a merge result with thousands of branches. | 21:04 |
| corvus | and in those case, forwarding by reference would probably be a big win. | 21:04 |
| clarkb | ya I suspect for our use cases its almost never an issue (otherwise we'd have errors today?) | 21:04 |
| clarkb | our == opendev | 21:05 |
| clarkb | other users may have different demands | 21:05 |
| corvus | clarkb: i think we actually do, but they're super rare and only related to big github blobs | 21:05 |
| corvus | so we never notice :) | 21:05 |
| corvus | i think i've talked myself into doing both things -- primarily shard-when-necessary; secondarily forward-by-reference | 21:06 |
| corvus | i will plan to do those as 2 separate follow-up patches if i can. hopefully we can review them separately, then we can decide if we want to squash or do a temp test timeout increase | 21:07 |
| corvus | the only-when-necessary change is noticably faster than even the start of the stack | 21:33 |
| corvus | okay, i don't think it's worth doing forward-by-reference at this point. the trigger queue needs the event data, so it will always need to read it back in. we could still save the second write, but at the cost of significant complexity -- and i don't think very many trigger events are going to be that big anyway. the result queue is likely to have large merge results, but it doesn't get forwarded -- we only have pipeline-specific result queues, | 21:46 |
| corvus | so there's no benefit there. | 21:46 |
| clarkb | corvus: I've left notes on the nodeset change can you take a look? | 21:46 |
| corvus | clarkb: replied. do you think 1787 and 2319 is worth a new ps? | 22:00 |
| clarkb | looking | 22:02 |
| clarkb | (I juts approved the fingergw config reorg) | 22:02 |
| clarkb | corvus: no I think separate changes are fine for both of those | 22:03 |
| opendevreview | James E. Blair proposed zuul/zuul master: Only shard events when necessary https://review.opendev.org/c/zuul/zuul/+/801540 | 22:04 |
| corvus | i think i should go ahead and squash that into the parent; it's pretty simple | 22:04 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801540 | 22:05 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 22:06 |
| corvus | the patchset delta there should be easy to review | 22:07 |
| clarkb | corvus: I think there is a bug in ^ details in review | 22:25 |
| corvus | clarkb: i'm not sure i see the bug | 22:45 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 22:46 |
| Clark[m] | corvus: data in this case is a python object not the byte stream we want to save | 22:47 |
| Clark[m] | encoded_data is the stream | 22:47 |
| corvus | Clark: ah thank you, sorry i misunderstood | 22:48 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard event queue data https://review.opendev.org/c/zuul/zuul/+/801402 | 22:48 |
| clarkb | corvus: that latest version lgtm | 23:21 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!