[GitHub] [phoenix] stoty commented on a change in pull request #1365: PHOENIX-6611 Multiple IndexTool bugs

[GitBox <gi...@apache.org>]

stoty commented on a change in pull request #1365:
URL: https://github.com/apache/phoenix/pull/1365#discussion_r771111661



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/execute/PhoenixTxIndexMutationGenerator.java
##########
@@ -364,7 +364,8 @@ private boolean generatePuts(
                 throw new IOException(e);
             }
         }
-        Iterable<IndexUpdate> puts = codec.getIndexUpserts(state, indexMetaData, regionStartKey, regionEndKey);
+        Iterable<IndexUpdate> puts =
+                codec.getIndexUpserts(state, indexMetaData, regionStartKey, regionEndKey, true);

Review comment:
       This is in the normal write path via PhoenixTransactionalIndexer, not the in the async index generation.
   
   Three-phase commit is not used in the normal TX write path, because the transactionality is taking care of the
   consistency, and accordingly it never updates the status to VERIFIED, and leaves the status as "x".
   
   This method is also used by the old indexing implementation, and I didn't want old-style tables to set the status to verified, hence the new parameter.
   
   The same basic problem pattern that BulkLoad and Async Indextool has, but this one is in the normal write path.
   
   I should have called this change out explicitly, as admittedly this has the bigggest impact in the patch, but forgot, sorry.
   
   This really should have been four patches (snapshot, Async VERIFIED, TX index status, and the directApi purge), but I got carried away as I kept finding the issues by the expanded test case.
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org