virajjasani commented on code in PR #2321:
URL: https://github.com/apache/phoenix/pull/2321#discussion_r2601800767
##########
phoenix-core-client/src/main/java/org/apache/phoenix/index/IndexMetaDataCacheClient.java:
##########
@@ -120,10 +129,58 @@ public static ServerCache
setMetaDataOnMutations(PhoenixConnection connection, P
txState = connection.getMutationState().encodeTransaction();
}
boolean hasIndexMetaData = indexMetaDataPtr.getLength() > 0;
+ ReadOnlyProps props = connection.getQueryServices().getProps();
if (hasIndexMetaData) {
- if (
+ List<PTable> indexes = table.getIndexes();
+ boolean sendIndexMaintainers = false;
+ if (indexes != null) {
+ for (PTable index : indexes) {
+ if (IndexMaintainer.sendIndexMaintainer(index)) {
+ sendIndexMaintainers = true;
+ break;
+ }
+ }
+ }
+ boolean useServerMetadata =
props.getBoolean(INDEX_USE_SERVER_METADATA_ATTRIB,
+ QueryServicesOptions.DEFAULT_INDEX_USE_SERVER_METADATA)
+ && props.getBoolean(QueryServices.INDEX_REGION_OBSERVER_ENABLED_ATTRIB,
+ QueryServicesOptions.DEFAULT_INDEX_REGION_OBSERVER_ENABLED);
+ boolean serverSideImmutableIndexes =
+ props.getBoolean(SERVER_SIDE_IMMUTABLE_INDEXES_ENABLED_ATTRIB,
+ DEFAULT_SERVER_SIDE_IMMUTABLE_INDEXES_ENABLED);
+ boolean useServerCacheRpc =
useIndexMetadataCache(connection, mutations,
indexMetaDataPtr.getLength() + txState.length)
+ && sendIndexMaintainers;
+ long updateCacheFreq = table.getUpdateCacheFrequency();
+ // PHOENIX-7727 Eliminate IndexMetadataCache RPCs by leveraging server
PTable cache and
+ // retrieve IndexMaintainer objects for each active index from the
PTable object.
+ // To optimize rpc calls, use it only when all of these conditions are
met:
+ // 1. Use server metadata feature is enabled (enabled by default).
+ // 2. New index design is used (IndexRegionObserver coproc).
+ // 3. Table is not of type System.
+ // 4. Either table has mutable indexes or server side handling of
immutable indexes is
+ // enabled.
+ // 5. Table's UPDATE_CACHE_FREQUENCY is not ALWAYS. This ensures
IndexRegionObserver
+ // does not have to make additional getTable() rpc call with each
batchMutate() rpc call.
+ // 6. Table's UPDATE_CACHE_FREQUENCY is ALWAYS but addServerCache() rpc
call is needed
+ // due to the size of mutations. Unless expensive addServerCache() rpc
call is required,
+ // client can attach index maintainer mutation attribute so that
IndexRegionObserver
+ // does not have to make additional getTable() rpc call with each
batchMutate() rpc call
+ // with small mutation size (size <
phoenix.index.mutableBatchSizeThreshold value).
+ // If above conditions do not match and if the mutation size is greater
than
+ // "phoenix.index.mutableBatchSizeThreshold" value, however if none of
the data table
+ // indexes need to be sent to server (only index in state other than
DISABLE,
+ // CREATE_DISABLE, PENDING_ACTIVE need to be sent to server), do not use
expensive
+ // addServerCache() rpc call.
Review Comment:
@tkhurana you have found one of the trickiest part of this work :)
This particular explanation is for a minor behavior change that should be
applicable to our codebase even without this patch. While `boolean
hasIndexMetaData = indexMetaDataPtr.getLength() > 0` will ensure the data table
mutation has some index mutation to send, the condition can still be true
sometimes, even if no index update is really needed. So far, i was able to
reproduce it for transform table with namespace enabled use case (though I
believe the transform table still has a long way to go before it can be
considered a reliable feature - mostly due to lack of funding).
Does this look good?
> If above conditions (mentioned before L170) are all false and data table
needs to send index mutation with the data table mutation, we use expensive
addServerCache() rpc call. However, if above conditions (mentioned before L170)
are all false and data table mutation does not need to send index mutation
(because all indexes are in any of DISABLE, CREATE_DISABLE, PENDING_ACTIVE
states), we can avoid expensive addServerCache() rpc call.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]