[
https://issues.apache.org/jira/browse/PHOENIX-4053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16106322#comment-16106322
]
Samarth Jain commented on PHOENIX-4053:
---------------------------------------
[~jamestaylor], patch looks good for the most part. Below is some initial
feedback I have:
In Indexer, it looks like we could be acquiring the lock for the same row
multiple times. While it may be functionally correct, it would be less wasteful
to acquire it only once after all the mutations for a row have been grouped:
{code}
// add the mutation to the batch set
ImmutableBytesPtr row = new ImmutableBytesPtr(m.getRow());
MultiMutation stored = mutations.get(row);
// we haven't seen this row before, so add it
if (stored == null) {
stored = new MultiMutation(row);
mutations.put(row, stored);
}
{code}
Preferably after this:
{code}
// early exit if it turns out we don't have any edits
if (mutations.isEmpty()) {
return;
}
{code}
I think if we do the above, then we can also get rid of the possibility that a
thread could acquire lock for the same row more than once. Is there some code
path other than above where this may not be true? If there is none then it
would make the implementation of LockManager simpler. If we don't plan to use
the LockManager class outside of the Indexer co-processor, I would make it a
private class too since it may not be appropriate to use elsewhere.
Also, since our locks are meant to be mutually exclusive, I think we can use a
ReentrantLock instead of ReentrantReadWriteLock.
> Lock row exclusively when necessary for mutable secondary indexing
> ------------------------------------------------------------------
>
> Key: PHOENIX-4053
> URL: https://issues.apache.org/jira/browse/PHOENIX-4053
> Project: Phoenix
> Issue Type: Bug
> Reporter: James Taylor
> Assignee: James Taylor
> Attachments: PHOENIX-4053_4.x-HBase-0.98_v2.patch,
> PHOENIX-4053_4.x-HBase-0.98_v3.patch, PHOENIX-4053_v2.patch,
> PHOENIX-4053_v3.patch, PHOENIX-4053_wip.patch
>
>
> From HBase 1.2 on, rows are not exclusively locked when the preBatchMutate
> call is made (see HBASE-18474). The mutable secondary index (global and
> local) depend on this to get a consistent snapshot of a row between the point
> when the current row value is looked up, and when the new row is written.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
