1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-15 19:32:40 +01:00
phorge-phorge/src/applications/config
epriestley 29fd3f136b Allow columns to be marked as nonmutable (so save() will not change them)
Summary:
Ref T6840. This feels a little dirty; open to alternate suggestions.

We currently have a race condition where multiple daemons may load a commit and then save it at the same time, when processing "reverts X" text. Prior to this feature, two daemons would never load a commit at the same time.

The "reverts X" load/save has no effect (doesn't change any object properties), but it will set the state back to the loaded state on save(). This overwrites any flag updates made to the commit in the meantime, and can produce the race in T6840.

In other cases (triggers, harbormaster, repositories) we deal with this kind of problem with "append-only-updates + single-consumer", or a bunch of locking. There isn't really a good place to add a single consumer for commits, since a lot of daemons need to access them. We could move the flags column to a separate table, but this feels pretty complicated. And locking is messy, also mostly because we have so many consumers.

Just exempting this column (which has unusual behavior) from `save()` feels OK-ish? I don't know if we'll have other use cases for this, and I like it even less if we never do, but this patch is pretty small and feels fairly understandable (that said, I also don't like that it can make some properties just silently not update if you aren't on the lookout).

So, this is //a// fix, and feels simplest/least-bad for the moment to me, I thiiink.

Test Plan: Added and executed unit tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6840

Differential Revision: https://secure.phabricator.com/D11822
2015-02-19 10:37:17 -08:00
..
application Remove getIconName from all applications 2015-01-30 12:11:21 -08:00
check Upgrade "masked" config to "hidden" 2015-02-13 10:59:50 -08:00
controller Upgrade "masked" config to "hidden" 2015-02-13 10:59:50 -08:00
custom Update callsites of phutil_json_decode. 2014-06-21 00:39:37 +10:00
editor Allow bin/config to affect database configuration and migrate between local and database configuration 2014-10-08 16:15:05 -07:00
exception Apply some autofix linter rules 2014-09-10 06:55:05 +10:00
issue Split Setup Issues into Groups 2015-02-10 12:53:00 -08:00
json Mask remaining config values, and implement set type 2013-01-16 15:06:07 -08:00
management Config - fix management scripts with --database parameter 2015-01-09 13:58:11 -08:00
option Allow columns to be marked as nonmutable (so save() will not change them) 2015-02-19 10:37:17 -08:00
phid Rename PHIDType classes 2014-07-24 08:05:46 +10:00
query Fix method visibility for PhabricatorPolicyAwareQuery subclasses 2015-01-14 07:01:16 +11:00
response Clean up a text string 2015-02-13 07:03:09 -08:00
schema Fix method visibility for PhabricatorConfigStorageSchema methods 2015-01-07 07:33:52 +11:00
storage Fix visiblity of LiskDAO::getConfiguration() 2015-01-14 06:54:13 +11:00
view Fix pht method calls 2015-02-10 18:57:45 +11:00