narendly commented on a change in pull request #1178:
URL: https://github.com/apache/helix/pull/1178#discussion_r470289078
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java
##########
@@ -550,17 +546,71 @@ private ZkClient getZkClient(String path) {
}
private String getZkRealm(String path) {
+ if (_routingDataUpdateOnCacheMissEnabled) {
+ try {
+ return updateRoutingDataOnCacheMiss(path);
+ } catch (InvalidRoutingDataException e) {
+ LOG.error(
+ "FederatedZkClient::getZkRealm: Failed to update routing data due
to invalid routing "
+ + "data!", e);
+ throw new MultiZkException(e);
+ }
+ }
+ return _metadataStoreRoutingData.getMetadataStoreRealm(path);
+ }
+
+ /**
+ * Perform a 2-tier routing data cache update:
+ * 1. Do an in-memory update from the singleton RoutingDataManager
+ * 2. Do an I/O based read from the routing data source by resetting
RoutingDataManager
+ * @param path
+ * @return
+ * @throws InvalidRoutingDataException
+ */
+ private String updateRoutingDataOnCacheMiss(String path) throws
InvalidRoutingDataException {
String zkRealm;
try {
zkRealm = _metadataStoreRoutingData.getMetadataStoreRealm(path);
- } catch (NoSuchElementException ex) {
- throw new NoSuchElementException("Cannot find ZK realm for the path: " +
path);
- }
-
- if (zkRealm == null || zkRealm.isEmpty()) {
- throw new NoSuchElementException("Cannot find ZK realm for the path: " +
path);
+ } catch (NoSuchElementException e1) {
+ synchronized (this) {
+ try {
+ zkRealm = _metadataStoreRoutingData.getMetadataStoreRealm(path);
+ } catch (NoSuchElementException e2) {
+ // Try 1) Refresh MetadataStoreRoutingData from RoutingDataManager
+ // This is an in-memory refresh from the Singleton
RoutingDataManager - other
+ // FederatedZkClient objects may have triggered a cache refresh, so
we first update the
+ // in-memory reference. This refresh only affects this
object/thread, so we synchronize
+ // on "this".
+ _metadataStoreRoutingData =
+
RealmAwareZkClient.getMetadataStoreRoutingData(_connectionConfig);
+ try {
+ zkRealm = _metadataStoreRoutingData.getMetadataStoreRealm(path);
+ } catch (NoSuchElementException e3) {
+ synchronized (FederatedZkClient.class) {
+ try {
+ zkRealm =
_metadataStoreRoutingData.getMetadataStoreRealm(path);
+ } catch (NoSuchElementException e4) {
+ if (shouldThrottleRead()) {
+ // If routing data update from routing data source has taken
place recently,
+ // then just skip the update and throw the exception
+ throw e4;
+ }
+ // Try 2) Reset RoutingDataManager and re-read the routing
data from routing data
+ // source via I/O. Since RoutingDataManager's cache doesn't
have it either, so we
+ // synchronize on all threads by locking on
FederatedZkClient.class.
+ RoutingDataManager.getInstance().reset();
Review comment:
I'm not sure if that's feasible. Each update has different logic if you
look closely. Also, I think if we change the structure, it runs the risk of
making it harder to read/understand. Do you have a more concrete suggestion?
Tying it to the number of tries isn't a good idea since this isn't about how
many times you try the same logic. In other words, this logic is not a retry
(although it looks like one).
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
