xs transaction

xs transaction

[Tian, Kevin]

Seems currently xenstore transaction provides confusing message 
in different places:

a) comment of xs_transaction_start says "You can only have one 
transaction at any time." However do_transaction_start allows up to 
10 transactions created, as long as all other existing transaction 
channel is idle at the time (conn->transaction == NULL)

b) when multiple transactions can be created, xenstore is not ready 
to handle traffic in parallel. Only one conn->transaction exists 

c) do_transaction_start says "/* We don't support nested transactions. 
*/" by checking conn->transaction, but it can't handle nest case. 
Conn->transaction only indicates whether traffic is on-going in other 
transaction, instead of whether other transactions are created. Instead 
conn->transaction_list should be checked for the purpose.

So what's expected usage for current xs transaction? Should xenstore 
add check on parallel/nest case, or depend on caller to avoid?

Thanks,
Kevin

Re: xs transaction

[Keir Fraser]

We do support concurrent transactions at the API level, but the
implementation will currently allow only one concurrent transaction that
modifies state to succeed. This is a detail of the implementation however
(from a semantic point of view -- it may impact performance of course!).

Nested transactions (transactions within transactions) are not supported,
and attempts to create a nested transaction will be explicitly failed.

 -- Keir

On 11/9/07 08:17, "Tian, Kevin" <kevin.tian@intel.com> wrote:

> Seems currently xenstore transaction provides confusing message
> in different places:
> 
> a) comment of xs_transaction_start says "You can only have one
> transaction at any time." However do_transaction_start allows up to
> 10 transactions created, as long as all other existing transaction
> channel is idle at the time (conn->transaction == NULL)
> 
> b) when multiple transactions can be created, xenstore is not ready
> to handle traffic in parallel. Only one conn->transaction exists
> 
> c) do_transaction_start says "/* We don't support nested transactions.
> */" by checking conn->transaction, but it can't handle nest case.
> Conn->transaction only indicates whether traffic is on-going in other
> transaction, instead of whether other transactions are created. Instead
> conn->transaction_list should be checked for the purpose.
> 
> So what's expected usage for current xs transaction? Should xenstore
> add check on parallel/nest case, or depend on caller to avoid?
> 
> Thanks,
> Kevin
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: xs transaction

[Tian, Kevin]

>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>Sent: 2007年9月11日 15:32
>
>We do support concurrent transactions at the API level, but the
>implementation will currently allow only one concurrent transaction that
>modifies state to succeed. This is a detail of the implementation however
>(from a semantic point of view -- it may impact performance of course!).
>
>Nested transactions (transactions within transactions) are not supported,
>and attempts to create a nested transaction will be explicitly failed.
>
> -- Keir
>

Sorry that I may miss some important logic here. However from what I 
read, do_transaction_start doesn't prevent nest case:

void do_transaction_start(struct connection *conn, struct buffered_data
*in)
{
	...
	/* We don't support nested transactions. */
	if (conn->transaction) {
		send_error(conn, EBUSY);
		return;
	}

Conn->transaction is only set in life cycle of each message process:
static void process_message(struct connection *conn, struct buffered_data *in)
{
	trans = transaction_lookup(conn, in->hdr.msg.tx_id);
	...
	conn->transaction = trans;
	switch (in->hdr.msg.type) {
	...
	conn->transaction = NULL;
}

So given following nest case, I'm not sure how it's prevented:
	Xs_transaction_start(xs_handle);
	<- at this point, conn->transaction is NULL
	Xs_transaction_start(xs_handle); 

Thanks,
Kevin

Re: xs transaction

[Keir Fraser]

On 11/9/07 08:34, "Tian, Kevin" <kevin.tian@intel.com> wrote:

> So given following nest case, I'm not sure how it's prevented:
> Xs_transaction_start(xs_handle);
> <- at this point, conn->transaction is NULL
> Xs_transaction_start(xs_handle);

These transactions are not nested (at least from the p.o.v. of xenstored):
they are independent transaction contexts which cannot see one another's
updates. It's not even possible to request construction of a nested
transaction via the interfaces exposed in xs.h.

 -- Keir

RE: xs transaction

[Tian, Kevin]

>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>Sent: 2007年9月11日 15:43
>
>On 11/9/07 08:34, "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
>> So given following nest case, I'm not sure how it's prevented:
>> Xs_transaction_start(xs_handle);
>> <- at this point, conn->transaction is NULL
>> Xs_transaction_start(xs_handle);
>
>These transactions are not nested (at least from the p.o.v. of xenstored):
>they are independent transaction contexts which cannot see one
>another's
>updates. It's not even possible to request construction of a nested
>transaction via the interfaces exposed in xs.h.
>

OK, if you mean nest as two transactions with parent-child relationship... 
So the problem is still here, existing code doesn't prevent multiple 
transactions created under same connection, while message process 
logic can't not handle multiple.

Say two threads both start transactions under same connection ID. It's 
likely to succeed as long as two starts goes in sequence. However later 
once two threads have message sent to xenstore at same time, one 
will cause assertion and xenstore can't handle.

Since we're sure that current implementation doesn't handle parallel 
transactions (though API level allows), I think do_transaction_start 
should check conn->transaction_list instead of conn->transaction to 
ensure it.

Thanks,
Kevin

Re: xs transaction

[Vincent Hanquez]

On Tue, Sep 11, 2007 at 03:44:24PM +0800, Tian, Kevin wrote:
> OK, if you mean nest as two transactions with parent-child relationship... 
> So the problem is still here, existing code doesn't prevent multiple 
> transactions created under same connection, while message process 
> logic can't not handle multiple.
> 
> Say two threads both start transactions under same connection ID. It's 
> likely to succeed as long as two starts goes in sequence. However later 
> once two threads have message sent to xenstore at same time, one 
> will cause assertion and xenstore can't handle.

xenstored is single threaded and the libxs accesses are serialed through
a mutex which prevent two threads to access the ring at the same time.
if it was not, you'ld have much more problems than having two
transactions at the same time.

xenstored will always complete a process_message function until it try
to read a new packet. at this point the conn->transaction is back to
NULL.

> Since we're sure that current implementation doesn't handle parallel 
> transactions (though API level allows), I think do_transaction_start 
> should check conn->transaction_list instead of conn->transaction to 
> ensure it.

how are you sure it doesn't handle parallel transactions ?

-- 
Vincent Hanquez

RE: xs transaction

[Tian, Kevin]

>From: Vincent Hanquez [mailto:vincent@xensource.com]
>Sent: 2007年9月11日 19:45
>
>xenstored is single threaded and the libxs accesses are serialed through
>a mutex which prevent two threads to access the ring at the same time.
>if it was not, you'ld have much more problems than having two
>transactions at the same time.
>
>xenstored will always complete a process_message function until it try
>to read a new packet. at this point the conn->transaction is back to
>NULL.

Thanks, and I see the point now.

>
>> Since we're sure that current implementation doesn't handle parallel
>> transactions (though API level allows), I think do_transaction_start
>> should check conn->transaction_list instead of conn->transaction to
>> ensure it.
>
>how are you sure it doesn't handle parallel transactions ?

My fault actually and it does handle. If multiple transactions are in 
parallel, 1st end will update its changes into global tdb with the rest 
failed (EAGAIN) to caller for retry. :-)

Thanks,
Kevin