Repository imported with some commits missing Herald step
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	epriestley
	Dec 31 2014, 6:28 PM

Description

Importing this repository:

https://gerrit.wikimedia.org/r/p/operations/puppet

...leaves about 10 commits missing the "Herald" step; these include:

rOPUP4cbebb710ebbb84ea608ae78a4b77f08e2d31780
rOPUP5370107635ef0685368535710eda866fe930735a
rOPUPaa5af1ee3a52e37498efa902ca5ae2810312d990
rOPUP45071cbbad9760eef69d117630128cce2b45e890
rOPUPfd5273a5fdbf6766b386a90a983fe4b7ab44b9e0
rOPUP586b07329677b9aa7ee9268b8641e84a480def4e
rOPUPebcb7cd844d8ed68245dacd127cc7af979f94aa3
rOPUP69f3b4ec4c0c091f3b4aafb4c031bd998258beaf

Re-running the message/change/owners/herald steps on these commits imports them without any issues, though, and the daemon logs aren't showing anything useful.

One thing that these commits share is that they're all mentioned later by other commits, so I suspect that may be a clue.

Revisions and Commits

rP Phabricator
	D11822	rP29fd3f136b88 Allow columns to be marked as nonmutable (so save() will not change them)
	D11823	rP6a60b8cb6f2b Set "importStatus" as nonmutable on save()

Related Objects

Mentioned In: T7310: Improve UX around bad repository credentials

Event Timeline

epriestley created this task.Dec 31 2014, 6:28 PM

epriestley raised the priority of this task from to Low.

epriestley updated the task description. (Show Details)

epriestley added a project: Diffusion.

epriestley added subscribers: epriestley, • chasemp.

Herald added a subscriber: qgil. · View Herald TranscriptDec 31 2014, 6:28 PM

epriestley: Oh, I thiiink it might be a race condition.
epriestley: Yeah.
epriestley: Your commits are different from mine.
epriestley: So I think what's happening is:
epriestley: Process A is parsing the message for the commit which mentions the problematic commit. It loads a copy of the commit object.
epriestley: Process B is parsing the herald rules for the mentioned commit. It sets the "herald is done" flag.
epriestley: Process A writes the mention and saves the object, reverting the "herald is done" flag.

Tenatively, the easiest fix for this might be to add a Lisk property attribute which stops it from updating the importStatus field during a normal save().

Otherwise we could add a bunch of locks.

epriestley claimed this task.Feb 19 2015, 3:09 PM

epriestley raised the priority of this task from Low to High.

epriestley added a project: Phacility.

epriestley moved this task from Backlog to v1 Open Beta on the Phacility board.

epriestley added a revision: D11822: Allow columns to be marked as nonmutable (so save() will not change them).Feb 19 2015, 3:32 PM

epriestley added a revision: D11823: Set "importStatus" as nonmutable on save().Feb 19 2015, 3:41 PM