[
https://issues.apache.org/jira/browse/HBASE-12751?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14702101#comment-14702101
]
stack commented on HBASE-12751:
-------------------------------
Reviewing latest version:
{code}
Seems weird that the region doesn't own its sequenceid. Notion is that now the
mvcc instance owns the sequenceid and that there is an mvcc instance per
region. Thats fine. Just need to draw it out some. For example here in mvcc
class javadoc it say:
Manages the read/write consistency. This provides...
Should be 'for the region'?
I did not realize that the append-to-WAL was in more than just the WAL append
method but it is also up here in processRowsWithLocks and elsewhere...That is a
little crazy. Any chance at unification in a follow-on in your opinion? They
differ by a little only and the steps are pretty involved....
Lots of good clean up in here. Better method namings in mvcc class.
We run through all edits twice, once to set OperationStatus.SUCCESS and then
later to do postPut/postDelete on CP. Could these be combined? CP only looks at
current batch so should be ok.
Need to work on case where edits may get into the WAL then we fail memstore
update or sync. They are in WAL... the edits might get replayed or replicated
though client may have gotten message they were rejected. Not an issue
introduced here...
When would sequenceid be non-null in below:
if (cell.getSequenceId() == 0) {
CellUtil.setSequenceId(cell, mvccNum);
}
Maybe this should be an exception so we can find cases where this is set
already? Perhaps in DLR when replay? Later we set sequenceid for these.
How is this addressed?
// TODO: If we are using FlushAllStoresPolicy, then this can make edits
visible from other
// stores while they are still in flight because the flush commit
marker will not contain
// flushes from ALL stores.
The comment is removed without comment. It looks like still a prob?
Why this long currentSeqId = mvcc.readPoint.get(); and not mvcc.getReadPoint?
Ain't this done already for you higher up when you come in for a region
operation?
// Make sure the row is inside of this region before getting the lock for
it.
checkRow(row, "row lock");
Would be good to avoid another set of compares.
// TODO(eclark): Should this be an array of fixed size to
// reduce the number of allocations on the write path?
// This could be equal to the number of handlers + a small number.
Sounds right. If more than outstanding handlers, something is wrong.
So, mvcc write number can be incremented in a few places for various reasons...
but looking in FSWALEntry#stampRegionSequenceId, order in WAL and into memstore
should be good.
writeCompactionMarker and writeRegionEventMarker in WALUtil are hard to follow
-- the intrinsic sequence manipulation. Makes sense if you dig but this stuff
is complicated (not you... just is)
{code}
Let me see why all the test failures.
I think we should get this in. Good cleanup, some optimization, and a couple
of good bug fixes including a dataloss.
> Allow RowLock to be reader writer
> ---------------------------------
>
> Key: HBASE-12751
> URL: https://issues.apache.org/jira/browse/HBASE-12751
> Project: HBase
> Issue Type: Bug
> Components: regionserver
> Affects Versions: 2.0.0, 1.3.0
> Reporter: Elliott Clark
> Assignee: Elliott Clark
> Fix For: 2.0.0, 1.3.0
>
> Attachments: HBASE-12751-v1.patch, HBASE-12751-v10.patch,
> HBASE-12751-v10.patch, HBASE-12751-v11.patch, HBASE-12751-v12.patch,
> HBASE-12751-v13.patch, HBASE-12751-v14.patch, HBASE-12751-v15.patch,
> HBASE-12751-v16.patch, HBASE-12751-v17.patch, HBASE-12751-v18.patch,
> HBASE-12751-v19.patch, HBASE-12751-v2.patch, HBASE-12751-v3.patch,
> HBASE-12751-v4.patch, HBASE-12751-v5.patch, HBASE-12751-v6.patch,
> HBASE-12751-v7.patch, HBASE-12751-v8.patch, HBASE-12751-v9.patch,
> HBASE-12751.patch
>
>
> Right now every write operation grabs a row lock. This is to prevent values
> from changing during a read modify write operation (increment or check and
> put). However it limits parallelism in several different scenarios.
> If there are several puts to the same row but different columns or stores
> then this is very limiting.
> If there are puts to the same column then mvcc number should ensure a
> consistent ordering. So locking is not needed.
> However locking for check and put or increment is still needed.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
