<p dir="ltr">Huge +1. I actually have some knowledge about how transactions work in Nailgun, and the saddest is that the worst parts are Network Managers and garbage code in various task helpers. Please add me to discussions on topic, I'm back from vacation on Thursday.</p>
<div class="gmail_quote">18 Май 2015 г. 18:17 пользователь "Alexander Kislitsky" <<a href="mailto:akislitsky@mirantis.com">akislitsky@mirantis.com</a>> написал:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Let's collect work items together:</div><div><ol><li>Introduce UnitOfWork for generated astute messages. All messages should be sent to Astute only after HTTP handler is finished. For example in ApplyChangesTaskManager we are casting messages during processing method '_execute_async_content'</li><li>All nailgun Task objects should implement single interface. Now we have 'execute', 'message', 'get_message_body' methods. Task should implement 'message' and not cast messages to Astute. Tasks main function is to serialize messages for Astute. Nothing more.</li><li>We should improve deadlock detecting in the Nailgun. We should trace not only order of tables in the locks chain but also ids of locked objects. Also deadlock detecting should consider update, delete queries.</li><li>Commit calls should be present only in the HTTP middleware. We shouldn't manipulate DB transactions inside business logic.<br></li></ol></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 6, 2015 at 6:32 PM, Igor Kalnitsky <span dir="ltr"><<a href="mailto:ikalnitsky@mirantis.com" target="_blank">ikalnitsky@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>> We don't need transactions, for example, in GET methods.<br>
<br>
</span>It doesn't matter whether want we or not. The SQLAlchemy implicitly<br>
starts transaction on first "select" query and it's ok. I mean,<br>
perhaps it's not ok, but definitely it won't lead to great performance<br>
degradation. A large number of projects prove this.<br>
<span><br>
> I propose to rid of complex data flows in our code. Code with 'commit'<br>
> call inside the the method should be split into independent units.<br>
<br>
</span>Agree. We should get rid of non-obvious and unexpected commits.<br>
<span><br>
> I like the solution with sending tasks to Astute at the end of handler<br>
> execution.<br>
<br>
</span>I don't know how much effort we should apply to implement this, but on<br>
first look it seems ok. I mean, we save "tasks" to send in some queue<br>
and then send them if and only iff HTTP handler reports success.<br>
Currently, we send it, but there are few places where HTTP handler may<br>
fail, report error and perform partial rollback (why partial? because<br>
we commit task before sending to Astute) and it looks weird. :(<br>
<div><div><br>
<br>
<br>
On Wed, May 6, 2015 at 1:22 PM, Alexander Kislitsky<br>
<<a href="mailto:akislitsky@mirantis.com" target="_blank">akislitsky@mirantis.com</a>> wrote:<br>
> I mean, that we should have explicitly wrapped http handlers. For example:<br>
><br>
> @transaction<br>
> def PUT(...):<br>
>   ...<br>
><br>
> We don't need transactions, for example, in GET methods.<br>
><br>
> I propose to rid of complex data flows in our code. Code with 'commit' call<br>
> inside the the method should be split into independent units.<br>
><br>
> I like the solution with sending tasks to Astute at the end of handler<br>
> execution.<br>
><br>
> On Wed, May 6, 2015 at 12:57 PM, Igor Kalnitsky <<a href="mailto:ikalnitsky@mirantis.com" target="_blank">ikalnitsky@mirantis.com</a>><br>
> wrote:<br>
>><br>
>> > First of all I propose to wrap HTTP handlers by begin/commit/rollback<br>
>><br>
>> I don't know what you are talking about, but we do wrap handlers in<br>
>> transaction for a long time. Here's the code<br>
>><br>
>> <a href="https://github.com/stackforge/fuel-web/blob/2de3806128f398d192d7e31f4ca3af571afeb0b2/nailgun/nailgun/api/v1/handlers/base.py#L53-L84" target="_blank">https://github.com/stackforge/fuel-web/blob/2de3806128f398d192d7e31f4ca3af571afeb0b2/nailgun/nailgun/api/v1/handlers/base.py#L53-L84</a><br>
>><br>
>> The issue is that we sometimes perform `.commit()` inside the code<br>
>> (e.g. `task.execute()`) and therefore it's hard to predict which data<br>
>> are committed and which are not.<br>
>><br>
>> In order to avoid this, we have to declare strict scopes for different<br>
>> layers. Yes, we definitely should base on idea that handlers should<br>
>> open transaction on the begin and close it on the end. But that won't<br>
>> solve all problems, because sometimes we should commit data before<br>
>> handler's end. For instance, commit some task before sending message<br>
>> to Astute. Such cases complicate the things.. and it would be cool if<br>
>> could avoid them by re-factoring our architecture. Perhaps, we could<br>
>> send tasks to Astute when the handler is done? What do you think?<br>
>><br>
>> Thanks,<br>
>> igor<br>
>><br>
>> On Wed, May 6, 2015 at 12:15 PM, Lukasz Oles <<a href="mailto:loles@mirantis.com" target="_blank">loles@mirantis.com</a>> wrote:<br>
>> > On Wed, May 6, 2015 at 10:51 AM, Alexander Kislitsky<br>
>> > <<a href="mailto:akislitsky@mirantis.com" target="_blank">akislitsky@mirantis.com</a>> wrote:<br>
>> >> Hi!<br>
>> >><br>
>> >> The refactoring of transactions management in Nailgun is critically<br>
>> >> required<br>
>> >> for scaling.<br>
>> >><br>
>> >> First of all I propose to wrap HTTP handlers by begin/commit/rollback<br>
>> >> decorator.<br>
>> >> After that we should introduce transactions wrapping decorator into<br>
>> >> Task<br>
>> >> execute/message calls.<br>
>> >> And the last one is the wrapping of receiver calls.<br>
>> >><br>
>> >> As result we should have begin/commit/rollback calls only in<br>
>> >> transactions<br>
>> >> decorator.<br>
>> ><br>
>> > Big +1 for this. I always wondered why we don't have it.<br>
>> ><br>
>> >><br>
>> ><br>
>> >> Also I propose to separate working with DB objects into separate lair<br>
>> >> and<br>
>> >> use only high level Nailgun objects in the code and tests. This work<br>
>> >> was<br>
>> >> started long time ago, but not finished yet.<br>
>> >><br>
>> >> On Thu, Apr 30, 2015 at 12:36 PM, Roman Prykhodchenko <<a href="mailto:me@romcheg.me" target="_blank">me@romcheg.me</a>><br>
>> >> wrote:<br>
>> >>><br>
>> >>> Hi folks!<br>
>> >>><br>
>> >>> Recently I faced a pretty sad fact that in Nailgun there’s no common<br>
>> >>> approach to manage transactions. There are commits and flushes in<br>
>> >>> random<br>
>> >>> places of the code and it used to work somehow just because it was all<br>
>> >>> synchronous.<br>
>> >>><br>
>> >>> However, after just a few of the subcomponents have been moved to<br>
>> >>> different processes, it all started producing races and deadlocks<br>
>> >>> which are<br>
>> >>> really hard to resolve because there is absolutely no way to predict<br>
>> >>> how a<br>
>> >>> specific transaction is managed but by analyzing the source code. That<br>
>> >>> is<br>
>> >>> rather an ineffective and error-prone approach that has to be fixed<br>
>> >>> before<br>
>> >>> it became uncontrollable.<br>
>> >>><br>
>> >>> Let’s arrange a discussions to design a document which will describe<br>
>> >>> where<br>
>> >>> and how transactions are managed and refactor Nailgun according to it<br>
>> >>> in<br>
>> >>> 7.0. Otherwise results may be sad.<br>
>> >>><br>
>> >>><br>
>> >>> - romcheg<br>
>> >>><br>
>> >>><br>
>> >>><br>
>> >>> __________________________________________________________________________<br>
>> >>> OpenStack Development Mailing List (not for usage questions)<br>
>> >>> Unsubscribe:<br>
>> >>> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>> >>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>> >>><br>
>> >><br>
>> >><br>
>> >><br>
>> >> __________________________________________________________________________<br>
>> >> OpenStack Development Mailing List (not for usage questions)<br>
>> >> Unsubscribe:<br>
>> >> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>> >> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>> >><br>
>> ><br>
>> ><br>
>> ><br>
>> > --<br>
>> > Łukasz Oleś<br>
>> ><br>
>> ><br>
>> > __________________________________________________________________________<br>
>> > OpenStack Development Mailing List (not for usage questions)<br>
>> > Unsubscribe:<br>
>> > <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>> > <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>><br>
>> __________________________________________________________________________<br>
>> OpenStack Development Mailing List (not for usage questions)<br>
>> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
><br>
><br>
> __________________________________________________________________________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div>