You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Ski Gh3 <sk...@gmail.com> on 2009/07/09 01:29:12 UTC
Questions on the transactional package
Hi,
I was browsing the transactional package to understand how it works, and got
a little confused and wonder if someone can help me out:
In the TransactionalRegion class, why is the commitPendingTransactions set
added to the current transaction in beginTransaction method?
shouldn't it be in the RequestCommit method?
plus, shouldn't this code be protected using synchronized, even though
commitPendingTransactions is a synchronized set, since this is a iteration?
the code is as follows:
// Order is important here
for (TransactionState commitPending : commitPendingTransactions) {
state.addTransactionToCheck(commitPending);
}
Thanks!
Re: Questions on the transactional package
Posted by Clint Morgan <cl...@troove.net>.
Yes the Impl is based on that paper, but we also have the distribution
concern (hence the pending list for the duration of the 2PC).
That case you mentioned is covered. If another trx is commitRequested after
current trx answers yes to commit request, then it (the other) must be
checked against the current. If the other trx is commitRequested before the
current trx, then we will have to check for confict agains it (will have
higer sequence number).
Make sense?
On Thu, Jul 9, 2009 at 1:27 AM, Ski Gh3 <sk...@gmail.com> wrote:
> To make sure I'm not missing sth, is the implementation based on the paper
> referred to in the package description (specifically Sec. 5)?
> If yes, then I believe that commitPending transactions should be copied
> when
> we are in commitRequest. otherwise a transaction started later then the
> current transaction, but committed after the requestCommit of the current
> transaction (yet before the current transaction commit) will not be checked
> (it escaped both check for committed transactions and for pending
> transactions). This may cause the current transaction to conflict with it.
> Just my two cents.
>
> On Wed, Jul 8, 2009 at 5:15 PM, Clint Morgan <clint.morgan@troove.net
> >wrote:
>
> > Those commitPending transactions have not been committed when the
> > transaction starts, but may be committed when the transaction is ready to
> > commit. Thus we will need to verify that we don't conflict with them (if
> > they commit first). Otherwise we could have read one of their rows before
> > they get committed and then violate transaciton atomicity.
> >
> > At commit time we figure out the rest of the transacitons that we need to
> > check against by looking though all transactions that have committed
> since
> > the current transaciton began. We use the sequence number to calculate
> > this.
> > The commitPending ones are added explicity because they have a sequence
> > number before our startSeqNum...
> >
> > You are right about threading/iteration issue. The current code iterates
> > over a copy of commitPendingTransactions to fix that potential CME. But
> > thinking on it there is still a race condition here, so a lock would be
> > needed. I'll look into it before 0.20...
> >
> > -clint
> >
> > On Wed, Jul 8, 2009 at 4:29 PM, Ski Gh3 <sk...@gmail.com> wrote:
> >
> > > Hi,
> > >
> > > I was browsing the transactional package to understand how it works,
> and
> > > got
> > > a little confused and wonder if someone can help me out:
> > >
> > > In the TransactionalRegion class, why is the commitPendingTransactions
> > set
> > > added to the current transaction in beginTransaction method?
> > > shouldn't it be in the RequestCommit method?
> > >
> > > plus, shouldn't this code be protected using synchronized, even though
> > > commitPendingTransactions is a synchronized set, since this is a
> > iteration?
> > >
> > > the code is as follows:
> > >
> > > // Order is important here
> > > for (TransactionState commitPending : commitPendingTransactions) {
> > > state.addTransactionToCheck(commitPending);
> > > }
> > >
> > > Thanks!
> > >
> >
>
Re: Questions on the transactional package
Posted by Ski Gh3 <sk...@gmail.com>.
To make sure I'm not missing sth, is the implementation based on the paper
referred to in the package description (specifically Sec. 5)?
If yes, then I believe that commitPending transactions should be copied when
we are in commitRequest. otherwise a transaction started later then the
current transaction, but committed after the requestCommit of the current
transaction (yet before the current transaction commit) will not be checked
(it escaped both check for committed transactions and for pending
transactions). This may cause the current transaction to conflict with it.
Just my two cents.
On Wed, Jul 8, 2009 at 5:15 PM, Clint Morgan <cl...@troove.net>wrote:
> Those commitPending transactions have not been committed when the
> transaction starts, but may be committed when the transaction is ready to
> commit. Thus we will need to verify that we don't conflict with them (if
> they commit first). Otherwise we could have read one of their rows before
> they get committed and then violate transaciton atomicity.
>
> At commit time we figure out the rest of the transacitons that we need to
> check against by looking though all transactions that have committed since
> the current transaciton began. We use the sequence number to calculate
> this.
> The commitPending ones are added explicity because they have a sequence
> number before our startSeqNum...
>
> You are right about threading/iteration issue. The current code iterates
> over a copy of commitPendingTransactions to fix that potential CME. But
> thinking on it there is still a race condition here, so a lock would be
> needed. I'll look into it before 0.20...
>
> -clint
>
> On Wed, Jul 8, 2009 at 4:29 PM, Ski Gh3 <sk...@gmail.com> wrote:
>
> > Hi,
> >
> > I was browsing the transactional package to understand how it works, and
> > got
> > a little confused and wonder if someone can help me out:
> >
> > In the TransactionalRegion class, why is the commitPendingTransactions
> set
> > added to the current transaction in beginTransaction method?
> > shouldn't it be in the RequestCommit method?
> >
> > plus, shouldn't this code be protected using synchronized, even though
> > commitPendingTransactions is a synchronized set, since this is a
> iteration?
> >
> > the code is as follows:
> >
> > // Order is important here
> > for (TransactionState commitPending : commitPendingTransactions) {
> > state.addTransactionToCheck(commitPending);
> > }
> >
> > Thanks!
> >
>
Re: Questions on the transactional package
Posted by Clint Morgan <cl...@troove.net>.
Those commitPending transactions have not been committed when the
transaction starts, but may be committed when the transaction is ready to
commit. Thus we will need to verify that we don't conflict with them (if
they commit first). Otherwise we could have read one of their rows before
they get committed and then violate transaciton atomicity.
At commit time we figure out the rest of the transacitons that we need to
check against by looking though all transactions that have committed since
the current transaciton began. We use the sequence number to calculate this.
The commitPending ones are added explicity because they have a sequence
number before our startSeqNum...
You are right about threading/iteration issue. The current code iterates
over a copy of commitPendingTransactions to fix that potential CME. But
thinking on it there is still a race condition here, so a lock would be
needed. I'll look into it before 0.20...
-clint
On Wed, Jul 8, 2009 at 4:29 PM, Ski Gh3 <sk...@gmail.com> wrote:
> Hi,
>
> I was browsing the transactional package to understand how it works, and
> got
> a little confused and wonder if someone can help me out:
>
> In the TransactionalRegion class, why is the commitPendingTransactions set
> added to the current transaction in beginTransaction method?
> shouldn't it be in the RequestCommit method?
>
> plus, shouldn't this code be protected using synchronized, even though
> commitPendingTransactions is a synchronized set, since this is a iteration?
>
> the code is as follows:
>
> // Order is important here
> for (TransactionState commitPending : commitPendingTransactions) {
> state.addTransactionToCheck(commitPending);
> }
>
> Thanks!
>