From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 98F1929851 for ; Thu, 30 Aug 2018 08:31:10 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aA7TTJKhy_kh for ; Thu, 30 Aug 2018 08:31:10 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id DE9C82983E for ; Thu, 30 Aug 2018 08:31:09 -0400 (EDT) Date: Thu, 30 Aug 2018 15:31:07 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 1/2] Update lua space cache just after creation Message-ID: <20180830123106.GB30997@chai> References: <52f6dd8a94f65030216f78796b8ed3d7f91f5eb2.1535472838.git.georgy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52f6dd8a94f65030216f78796b8ed3d7f91f5eb2.1535472838.git.georgy@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Georgy Kirichenko * Georgy Kirichenko [18/08/28 19:20]: > The lua space cache (box.space.*) should be valid just after space is > created because space is ready to accept new records or does not exists > before wal would be written. So invoke a space create/drop trigger after > a space is changed and recall it in a case of rollback. I understand what this patch is doing, but I don't understand why you need it. We need to alert the user about system events, such as newly created spaces, including system spaces. 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? 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? 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. 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. > @@ -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, -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov