[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4553: Add integration test for ControllerLeaderLocator
jackjlli commented on a change in pull request #4553: Add integration test for
ControllerLeaderLocator
URL: https://github.com/apache/incubator-pinot/pull/4553#discussion_r317334696
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -80,26 +81,27 @@ public static ControllerLeaderLocator getInstance() {
/**
* Locates the controller leader so that we can send LLC segment completion
requests to it.
- * Checks the {@link
ControllerLeaderLocator::_cachedControllerLeaderInvalid} flag and fetches the
leader from helix if cached value is invalid
+ * Checks the {@link
ControllerLeaderLocator::_cachedValidControllerLeaderMap} and fetches the
leader from helix if cached value is invalid or missing.
* @param rawTableName table name without type.
* @return The host-port pair of the current controller leader.
*/
public synchronized Pair getControllerLeader(String
rawTableName) {
-if (!_cachedControllerLeaderInvalid) {
- return _controllerLeaderHostPort;
+Pair leaderPairForTable =
_cachedValidControllerLeaderMap.get(rawTableName);
Review comment:
Good point. Updated.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4553: Add integration test for ControllerLeaderLocator
jackjlli commented on a change in pull request #4553: Add integration test for
ControllerLeaderLocator
URL: https://github.com/apache/incubator-pinot/pull/4553#discussion_r317334710
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -147,7 +149,7 @@ private boolean isLeadControllerResourceEnabled() {
if (leaderHostAndPortPair != null) {
return leaderHostAndPortPair;
} else {
- LOGGER.warn("Could not locate leader for table: {}", rawTableName);
+ LOGGER.warn("Could not locate leader for Table: {}", rawTableName);
Review comment:
Reverted.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4553: Add integration test for ControllerLeaderLocator
jackjlli commented on a change in pull request #4553: Add integration test for
ControllerLeaderLocator
URL: https://github.com/apache/incubator-pinot/pull/4553#discussion_r317334621
##
File path:
pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
##
@@ -221,30 +223,31 @@ private boolean isLeadControllerResourceEnabled() {
}
/**
- * Invalidates the cached controller leader value by setting the {@link
ControllerLeaderLocator::_cacheControllerLeadeInvalid} flag.
+ * Invalidates the cached controller leader value by removing the existing
pair from {@link ControllerLeaderLocator::_cachedValidControllerLeaderMap}.
* This flag is always checked first by {@link
ControllerLeaderLocator::getControllerLeader()} method before returning the
leader. If set, leader is fetched from helix, else cached leader value is
returned.
*
* Invalidates are not allowed more frequently than {@link
ControllerLeaderLocator::MILLIS_BETWEEN_INVALIDATE} millis.
* The cache is invalidated whenever server gets NOT_LEADER or NOT_SENT
response. A NOT_LEADER response definitely needs a cache refresh. However, a
NOT_SENT response could also happen for reasons other than controller not being
leader.
* Thus the frequency limiting is done to guard against frequent cache
refreshes, in cases where we might be getting too many NOT_SENT responses due
to some other errors.
+ * @param rawTableName raw table name.
*/
- public synchronized void invalidateCachedControllerLeader() {
+ public synchronized void invalidateCachedControllerLeader(String
rawTableName) {
Review comment:
Good point. Actually the CRUSHed algorithm used for lead controller resource
guarantees least moves of the partition assignment change. It's more likely
that one partition leader won't affect other partition leader.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
