Re: [PATCH v2 01/13] tools/xenstore: don't allow creating too many nodes in a transaction

[Juergen Gross <jgross@xxxxxxxx>]

On 21.02.23 23:36, Julien Grall wrote:
> Hi Juergen,
>
> On 21/02/2023 08:10, Juergen Gross wrote:
> > On 20.02.23 19:01, Julien Grall wrote:
> > > So I have recreated an XTF test which I think match what you wrote [1].
> > >
> > > It is indeed failing without your patch. But then there are still some weird 
> > > behavior here.
> > > I would expect the creation of the node would also fail if instead of 
> > > removing the node if removed outside of the transaction.
> > > This is not the case because we are looking at the current quota. So 
> > > shouldn't we snapshot the global count?
> > As we don't do a global snapshot of the data base for a transaction (this was
> > changed due to huge memory needs, bad performance, and a higher transaction
> > failure rate), 
> I am a bit surprised that the only way to do proper transaction is to have a 
> global snapshot. Instead, you could have an overlay.
I didn't say that a global snapshot is the only way. And we are using an
overlay.

> > I don't think we should snapshot the count either.
> But that would mean that the quota will change depending on modification of the 
> global database while the transaction is inflight.
I really don't see the problem with that. But it seems our views are different
in this case.

> I guess this is not better nor worse that the current situation. But it is still 
> really confusing for a client because:
>    1) The error could happen at random point
Yes, like without a transaction.

>    2) You may see an inconsistent database as nodes are only cached when they 
> are first accessed
It isn't inconsistent at all. The same could happen if such other nodes are
added/modified/removed just a microsecond before you start the transaction.
You won't be able to tell the difference. You can only reason about nodes
being accessed in the transaction, and those are transaction-local.

> > A transaction is performed atomically at the time it is finished. Therefore
> > seeing the current global state inside the transaction (with the transaction
> > private state on top like an overlay) is absolutely fine IMO.
> To me it is just showing that our concept of transaction is very broken in C 
> Xenstored. I am curious to know whether OXenstored is behaving the same way.
I don't know either.

> Anyway, I agree this is not better nor worse than the current situation. So I 
> will acked this patch:
> Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature