<div dir="ltr">I think the key is to keep the transactions small. If you notice, a lot of the volume allocations pass *db to the functions so they can use the db when needed. Also, none of the calls in a Tx context call out to glusterfs because it would be time consuming and single threaded. Instead state is saved on the db, the tx is finished, and the work can the be started on gluster. If there is an error or the db needs to be update, it will do it after the completion of the work, which could have (and most likely was) done in parallel.<div><br></div><div>I think for the PR proposed, the SetState() call could take *db instead of *tx. If it is the simple ones (enable/disable) then the change can be done, and the function returns.</div><div><br></div><div>If it is "remove", then state of the device in the db can be changed, then db can be released and work can be started on that device. Then SetState() can return to the asynchttp routine with a result. The key thing here is that all bricks used to replace the bricks on the removed device where replaced in parallel.</div><div><br></div><div>- Luis</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 9, 2017 at 5:03 PM, Raghavendra Talur <span dir="ltr"><<a href="mailto:rtalur@redhat.com" target="_blank">rtalur@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Mar 10, 2017 at 3:21 AM, Michael Adam <<a href="mailto:obnox@samba.org">obnox@samba.org</a>> wrote:<br>
> Hi Talur,<br>
><br>
> Thanks for this write-up!<br>
><br>
> It sounds all very consistent.<br>
><br>
> But I will want to thoroughly look at the code.<br>
> The Mutex thing sounds a bit like complete<br>
> serialization (for write processes).<br>
<br>
</span>Yes, for small functions, this would completely serialize the<br>
goroutines and essentially we lose all the benefits. I could not come<br>
up with any better solution though.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Cheers - Michael<br>
><br>
> On 2017-03-10 at 02:03 +0530, Raghavendra Talur wrote:<br>
>> Here is what Bolt documentation says about transactions:<br>
>><br>
>> ```<br>
>> Transactions<br>
>><br>
>> Bolt allows only one read-write transaction at a time but allows as<br>
>> many read-only transactions as you want at a time. Each transaction<br>
>> has a consistent view of the data as it existed when the transaction<br>
>> started.<br>
>><br>
>> Individual transactions and all objects created from them (e.g.<br>
>> buckets, keys) are not thread safe. To work with data in multiple<br>
>> goroutines you must start a transaction for each one or use locking to<br>
>> ensure only one goroutine accesses a transaction at a time. Creating<br>
>> transaction from the DB is thread safe.<br>
>> ```<br>
>><br>
>> Now, in heketi, there some operations at app.* level that might have<br>
>> to view/update many buckets(node,brick,volumes,<wbr>device). The methods of<br>
>> these objects take either a *bolt.Tx or a *bolt.DB.<br>
>><br>
>> In my understanding, there are some app.* operations where it is a<br>
>> must that all the methods that are called within it, should get the<br>
>> same view of db; in that case, it makes sense that app.* function<br>
>> starts a tx and hands it down to the lower methods thereby ensuring a<br>
>> consistent view.<br>
>><br>
>> Some methods however, like Create and Destroy methods of brick have<br>
>> been written to take a *bolt.DB as a argument. This seems to be done<br>
>> to enable use of goroutines. Now, we have functions in the higher<br>
>> layer calling these very methods which would want the operation to be<br>
>> done in the transaction created above.<br>
>><br>
>> In PR #710 I have made two changes to tackle the above problem:<br>
>> 1. Changed these methods to take *bolt.Tx as arg instead of *bolt.DB<br>
>> 2. Introduce a sync.Mutex as another member of StatusGroup, any<br>
>> goroutines that want to operate on the db using a shared tx must call<br>
>> the new Lock() method of StatusGroup and call Unlock() after the db<br>
>> operation is done.<br>
>><br>
>> In summary, as bolt documentation suggests, to work with data in<br>
>> multiple go routines:<br>
>> we used to: start a transaction for each one<br>
>> now we do: mutex locking to ensure only one goroutine accesses a<br>
>> transaction at a time<br>
>><br>
>> If there are better ideas, they are most welcome.<br>
>><br>
>> Thanks,<br>
>> Raghavendra Talur<br>
>> ______________________________<wbr>_________________<br>
>> heketi-devel mailing list<br>
>> <a href="mailto:heketi-devel@gluster.org">heketi-devel@gluster.org</a><br>
>> <a href="http://lists.gluster.org/mailman/listinfo/heketi-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/heketi-devel</a><br>
______________________________<wbr>_________________<br>
heketi-devel mailing list<br>
<a href="mailto:heketi-devel@gluster.org">heketi-devel@gluster.org</a><br>
<a href="http://lists.gluster.org/mailman/listinfo/heketi-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/heketi-devel</a><br>
</div></div></blockquote></div><br></div>