[ https://issues.apache.org/jira/browse/PHOENIX-4053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16107602#comment-16107602 ]
Samarth Jain commented on PHOENIX-4053: --------------------------------------- [~jamestaylor], Regarding your comment here: {code} + if (!success) { + // We're throwing here, so we won't be locking any more rows. By setting the + // status to FAILURE, we prevent the attempt to unlock rows we've never + // locked when postBatchMutateIndispensably is executed. We're very + // limited about the state that can be shared between the batch mutate + // coprocessor calls (see HBASE-18482). + // Note that we shouldn't necessarily be throwing here, since we're + // essentially failing the data write because we can't do the locking + // necessary for performing consistent index maintenance. We'd ideally + // want to go through the index failure policy to determine what action + // to perform. We currently cannot ignore this lock failure + for (int j = i; j < miniBatchOp.size(); j++) { + miniBatchOp.setOperationStatus(j,FAILURE); + } + } {code} I don't see a throw statement here. My guess is there is some code in HBase which is going through the operation status array and taking appropriate action? I think it would make sense for the comment to state that. FWIW, I see callers of region.batchMutate within our Phoenix co-processors (UngroupedAggregateRegionObserver#commitBatch() and UngroupedAggregateRegionObserver#rebuildIndices) that don't seem to be looking at the return status. Maybe they should? > 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-4.x-HBase-0.98_v4.patch, > PHOENIX-4053-4.x-HBase-0.98_v6.patch, PHOENIX-4053_v2.patch, > PHOENIX-4053_v3.patch, PHOENIX-4053_v4.patch, PHOENIX-4053_v5.patch, > PHOENIX-4053_v6.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, > until the mvcc is advanced. Otherwise, a subsequent update to a row may not > see the current row state. Even with pre HBase 1.2 releases, the lock isn't > held long enough for us. We need to hold the locks from the start of the > preBatchMutate (when we read the data table to get the prior row values) > until the mvcc is advanced (beginning of postBatchMutateIndispensably). > Given the above, it's best if Phoenix manages the row locking itself > (mimicing the current HBase mechanism). -- This message was sent by Atlassian JIRA (v6.4.14#64029)