phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anoop Sam John (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PHOENIX-1245) Remove usage of empty KeyValue object BATCH_MARKER from Indexer
Date Wed, 10 Sep 2014 04:10:29 GMT

    [ https://issues.apache.org/jira/browse/PHOENIX-1245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14128021#comment-14128021
] 

Anoop Sam John commented on PHOENIX-1245:
-----------------------------------------

bq.If we remove usage of empty KeyValue object BATCH_MARKER from Indexer, does this mean we'll
no longer work with 0.98.1 - 0.98.5?
I don't think so James and Jesse.
I checked the Indexer code in 4.0 and master branch. Here is how it is the usage of empty
KV
{code}
private static KeyValue BATCH_MARKER = new KeyValue();
....
....
public void preSingleUpdate(final ObserverContext<RegionCoprocessorEnvironment> c, final
Mutation put,
      final WALEdit edit, final Durability durability) throws IOException {
      // just have to add a batch marker to the WALEdit so we get the edit again in the batch
      // processing step. We let it throw an exception here because something terrible has
happened.
      edit.add(BATCH_MARKER);
  }
....
....
public void preBatchMutateWithExceptions(ObserverContext<RegionCoprocessorEnvironment>
c,
      MiniBatchOperationInProgress<Mutation> miniBatchOp) throws Throwable {
...
// remove the batch keyvalue marker - its added for all puts
      WALEdit edit = miniBatchOp.getWalEdit(i);
      // we don't have a WALEdit for immutable index cases, which still see this path
      // we could check is indexing is enable for the mutation in prePut and then just skip
this
      // after checking here, but this saves us the checking again.
      if (edit != null) {
        KeyValue kv = edit.getKeyValues().get(0);
        if (kv == BATCH_MARKER) {
          // remove batch marker from the WALEdit
          edit.getKeyValues().remove(0);
        }
      }
...
{code}
You can see the object is added to WALEdit in one CP hook and in another it is just removed.
And this is private instance in this class so no other places this is being used. If there
were some code path which checks for the presence of this object in WALEdit and have diff
code flows, it would have been needed. (well that is what the 3.0 branch code doing I believe)
So if the present code in 4.0 and master branches can work with 0.98.1-0.98.5 versions of
HBase, after the suggested removal also, it can work with all these older version.
Pls note that the change in HBase is making such that the above remove attempt of the empty
KV from WALEdit is NOT removing. And that is why finally we get NPE down the line from HBase
code.

As per the discussion we are having in HBASE-11805 , we will restore the old behavior to 0.98.
 That is a must for Phoenix even if we remove the empty KV as proposed here.  Because  Phoenix
4.0, 4.1 versions should work with HBase 0.98.7+ right?  So we will get issues there. That
is why we will restore the old behavior in HBase 0.98

Still I suggest we can remove this unwanted op now.

Hope I am making it clear now. What do u say [~giacomotaylor], [~jesse_yates]

> Remove usage of empty KeyValue object BATCH_MARKER from Indexer
> ---------------------------------------------------------------
>
>                 Key: PHOENIX-1245
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1245
>             Project: Phoenix
>          Issue Type: Improvement
>    Affects Versions: 4.0.0, 5.0.0
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>            Priority: Critical
>             Fix For: 5.0.0, 4.2
>
>
> This is added to WALEdit in one CP hook and removed in a later CP hook. But not really
used.
> Now after HBASE-11805, the removal won't really happen. Later this empty KV is used in
other part of HBase core code and resulting in NPE as the bytes ref in this KV is null.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message