phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From JamesRTaylor <...@git.apache.org>
Subject [GitHub] phoenix pull request #290: PHOENIX-4130 Avoid server retries for mutable ind...
Date Fri, 26 Jan 2018 18:23:27 GMT
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/290#discussion_r164171049
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java ---
    @@ -312,6 +317,16 @@ private static QueryPlan addPlan(PhoenixStatement statement, SelectStatement
sel
             return null;
         }
     
    +    // returns true if we can still use the index
    +    // retuns false if we've been in PENDING_DISABLE too long - index should be considered
disabled
    +    private static boolean isUnderPendingDisableThreshold(PhoenixStatement statement,
PTable indexTable) {
    +        return EnvironmentEdgeManager
    +                .currentTimeMillis() - indexTable.getIndexDisableTimestamp() <= statement
    --- End diff --
    
    This runs on the client-side, but the INDEX_DISABLED_TIMESTAMP was set on the server-side,
so we can't necessarily reliably compare them. Instead, we should store the client timestamp
when we first received the exception and compare the client-side current time with that. Perhaps
a good place to store this pendingDisableInitialTimestamp would be on the index TableRef which
would not be shared across multiple threads.
    
    Also, minor nit: whenever possible (if on client-side), use .getConnection().getQueryServices().getProps()
instead of going through Configuration. We created this ReadOnlyProps which is essentially
a copy of Configuration but as an read-only, immutable map because we were seeing contention
on the locking done by Configuration (which is not necessary because it's used in an immutable
way). You can also add an indexPendingDisableThreshold as a member variable to QueryOptimizer.


---

Mime
View raw message