<div dir="ltr">I wrote a response inline. <div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Apr 24, 2017 at 3:12 PM, Jeff Ortel <<a href="mailto:jortel@redhat.com" target="_blank">jortel@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 04/21/2017 01:16 PM, Austin Macdonald wrote: > I think we have arrived at a consensus around these points: +1 > > * Django's ModelForm validations that are used during `full_clean` is an anti-pattern in DjangoRestFramework > and should not be used > * Validation, by our definition, will be done by the serializers in the API layer > * The Task layer should run business logic checks at the time of use > > We have not arrived at a consensus on whether Updates/Deletes for Importers/Publishers will be synchronous or > asynchronous. I've tried to think through each way, and what it would need to work. > > *Synchronous Update and Delete: > * > _Validation Heuristic_ > The serializers validate in the API layer only, and it is desirable to have that validation occur at the time > of the write. > * > * > _Concurrent Writes:_ > Let's assume synchronous Updates and Deletes. There is a possibility of concurrent writes because sync/publish > tasks write to the importer/publisher tables respectively and these tables can also be written by the Update > API at any time. If there is a collision the last write wins. > > Workaround: > Concurrent writes may not be a big deal because they should be not affect each other. In platform, sync and > publish only write to the last_sync/last_publish fields, which should never be written by the update API. If > the writes are restricted to only the affected fields, then there would be no clobbering. As long as plugin > writers' implementations of sync and publish do not write to the same fields allowed in an API update, this > concurrency is safe. We would obviously need to be clearly documented for plugin writers. > > _Dealing With Broken Configurations_ > For example, while a sync task is WAITING, the user updates an importer in a way that breaks sync. The > importer configuration should be read only once at the start of a sync. If this configuration has been deleted > or is not syncable, business logic checks catch this and fail immediately. Even if the configuration is > changed during the sync task, the sync task will continue with the original configuration. The sync task will > update last_sync, but will not write to other fields. I don't see a problem here, as long as this is > documented for the plugin writer. > > _Summary_: > I don't see any concurrency problems here. The downside of this design is that sync and publish, (implemented > by plugin writers), should not write to the same fields as API updates. > > * > * > *Asynchronous Update and Delete* > > _Validation Heuristic_ > Assuming async updates, the data comes into the API layer where it is validated by the serializers. The update > reserves a task (reservation is based on repository) and waits to be performed. It is usually undesirable to > separate validation from use. There could be concern that data validated in the API layer could become > unacceptable by the time the Task layer actually performs the write. > > Practically, this separation may not be an issue. Valid data becoming invalid involves validations that are > dependent on state. We can split these validations into two categories, uniqueness validations and multi-field > validations. Fortunately, the Django ORM will enforce uniqueness validation, so the database is protected, but > we might have to handle write failures. Multi-field validations are probably not validations by our > definition, but are business logic, which could be checked in the Task layer. > _ > _ > _Concurrent write prevention_ > Leveraging the reserved resource system, concurrent writes could not be a problem because writes could never > be concurrent at all. Syncs, Updates, Deletes, Publishes all make reservations based on the repository (this > is an assumption based on what we do in Pulp 2), so each would run in the order that they were called. > > _User Controlled Order_ > A user can issue a [sync][update][sync] and be confident that the first sync will use the original importer > configuration, and the second sync will use the updated configuration. This is not deterministic (from user point of view) even with task reservation. You still have a race condition queuing tasks. In the following example, the update made by user B will win and the update made by user A will not be used for the 2nd sync. By making this async, seems we have done is moved the race condition from the DB to the tasking queue. queue ------- 1. user-A [sync] 2. user-A [update] 3. user-B [update] (winner) 4. user-A [sync] </blockquote><div> </div><div>I'm not concerned about ^ because eventually Pulp3 will get authZ features which will allow administrators to prevent multi-user write-write races. Two users competing to write shared data is a normal, multi-user system problem. Also with the User A and User B update example, both synchronous and asynchronous update approaches would produce the same outcomes, so I don't think this point motivates either one as being better. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > > _Uncertain State_ > The configuration of an importer is unpredictable if there is a waiting update task from another user. > > > > > > > <div class="HOEnZb"><div class="h5">> _______________________________________________ > Pulp-dev mailing list > <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a> > <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> > </div></div> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </blockquote></div> </div></div></div>