Colin Watson has proposed merging
~cjwatson/launchpad:builder-set-preload-processors-robustness into
launchpad:master.
Commit message:
Make BuilderSet.preloadProcessors more robust
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/395781
We occasionally see buildd-manager log entries like this:
Failure while updating builders:
Traceback (most recent call last):
File ".../lib/lp/buildmaster/manager.py", line 755, in scanBuilders
self.builder_factory.update()
File ".../lib/lp/buildmaster/manager.py", line 274, in update
[b for b, _ in builders_and_current_bqs])
File ".../lib/lp/buildmaster/model/builder.py", line 287, in
preloadProcessors
cache._processors_cache.append(store.get(Processor, pid))
AttributeError: 'DefaultPropertyCache' object has no attribute
'_processors_cache'
The oldest occurrence of this on dogfood was 2020-10-01, and the oldest
occurrence on production was on 2020-10-10 (though there may have been some
older occurrences on production that have expired from logs). The frequency is
quite low: 11 occurrences on production including the first
on 2020-10-10.
It seems theoretically possible for the Storm cache to return a different
object from `store.get(Builder, bid)` than the Builder object with the same ID
that was passed into `BuilderSet.preloadProcessors`, although it's not quite
clear exactly how and I haven't been able to reproduce it in the test suite.
However, it's straightforward enough to guarantee to update the processors
cache on the objects that were passed in regardless of whatever Storm cache
shenanigans may occur, so do so.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:builder-set-preload-processors-robustness into
launchpad:master.
diff --git a/lib/lp/buildmaster/model/builder.py b/lib/lp/buildmaster/model/builder.py
index 5e66baf..a5f23bd 100644
--- a/lib/lp/buildmaster/model/builder.py
+++ b/lib/lp/buildmaster/model/builder.py
@@ -273,17 +273,17 @@ class BuilderSet(object):
# Grab (Builder.id, Processor.id) pairs and stuff them into the
# Builders' processor caches.
store = IStore(BuilderProcessor)
- builder_ids = [b.id for b in builders]
+ builders_by_id = {b.id: b for b in builders}
pairs = list(store.using(BuilderProcessor, Processor).find(
(BuilderProcessor.builder_id, BuilderProcessor.processor_id),
BuilderProcessor.processor_id == Processor.id,
- BuilderProcessor.builder_id.is_in(builder_ids)).order_by(
+ BuilderProcessor.builder_id.is_in(builders_by_id)).order_by(
BuilderProcessor.builder_id, Processor.name))
load(Processor, [pid for bid, pid in pairs])
for builder in builders:
get_property_cache(builder)._processors_cache = []
for bid, pid in pairs:
- cache = get_property_cache(store.get(Builder, bid))
+ cache = get_property_cache(builders_by_id[bid])
cache._processors_cache.append(store.get(Processor, pid))
def getBuilders(self):
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
