On Thursday, August 30, 2018 3:31:07 PM MSK Konstantin Osipov wrote: > We can well use txn->on_commit trigger for this purpose, having > set an on_commit trigger which pushes on_ctl events. This trigger > is invoked *after* a record is written to wal in txn.cc. Why do > you need to manipulate with the internal alter.cc triggers at all? > Why is the order of invocation of these triggers important? No, we can't, because generally there may be a space writes between space was created and space was commited. It is only current implementation issue that index creation acquires a ddl lock and stops applying. > > Ideally, your on_ctl trigger should be invoked after all the alter > triggers ahve been invoked. For that, all you need to do is to > ensure that on_ctl trigger is added *after* the alter trigger - > i.e. it has a lower priority/order than alter trigger. Is your > problem with trigger ordering? Is it that you've been trying to > add alter trigger *after* you added the on_ctl trigger, and, since > triggers are normally added to the end of the list, when on_ctl > trigger was invoked the space was not yet available in box.space? My problem I can't see lua space object for a created space while on_replace fires because on_replace trigger fires at operation stage and lua cache updates only after commit - this is the issue. This can't be solved with trigger ordering cos triggers are running on different stages and the only way i see is to move space cache update trigger to the operation stage. > > If it is the case, then I think we should simply add > trigger_add_first() function, and use this function to add alter > triggers to the beginning of the list. First we should change stage of lua cache updates, that the patch does. > > That would solve the problem until we have more strict ordering > requirements, at which point we could add trigger->order member > and trigger_add_with_order() method. > > Finally, I don't see how the test case is testing the order. I > believe the test case should pass both before and after the patch. Test uses fiber scheduling mechanism, the first fiber creates the second one that creates a space and yields on wal. The first fiber checks that corresponding space objects is visible (the second fiber still waiting for a wal). > > > @@ -1571,3 +1571,64 @@ fio = require('fio') > > > > box.space.test:drop() > > --- > > ... > > > > +-- allocate a space id to prevent max space id update > > Why is test description missing in the result file? > > > Nitpick: please add leading -- and trailing -- to the test > description, start it with a capital letter and end with a dot. > > > Thanks for the patch,