[server][common] Allow store config repo to fetch configs only when needed

[adamxchen]

Summary

#1132 introduced additional ZK watch. The implementation of HelixReadOnlyStoreConfigRepository reads all store configs, regardless of which cluster, this creates an issue since it would increase the additional ZK watch by store number x host name. In this PR, it adds some logics to lazy fetch the store config and cache.

How was this PR tested?

new unit test

Does this PR introduce any user-facing changes?

• No. You can skip the rest of this section.
• Yes. Make sure to explain your proposed changes and call out the behavior change.

[gaojieliu]

I am thinking the following refactoring strategy, which I think will be safer:

1. Keep the existing refresh calls as today.
2. HelixReadOnlyStoreConfigRepository#refresh will place a child-change listener as today, but it will only get a list of store names, not store configs.
3. In the child-change listener, whenever there is a child change, it will only mutate the maintained store list and of coz, for deleted stores, we will remove the corresponding entry from the local store config map and corresponding zk watch.
4. We will only place a zk watch on store config node in lazy fetch function.
5. In HelixReadOnlyStoreConfigRepository#getStoreConfig function, if the requested store doesn't in the store list, return Optional.empty() directly, and if it is present, fetch the store config on the fly and place the zk watch to monitor data change.
6. HelixReadOnlyStoreConfigRepository would maintain a storeConfigMap, but this map will be lazily populated by HelixReadOnlyStoreConfigRepository#getStoreConfig function.

WDYT?

[gaojieliu]

Left some more comments and you need to fix the test failures...

[gaojieliu]

Why this change?

[adamxchen]

It provides a bit better error handling, though essentially they are same.

  public static <T> List<String> listPathContents(ZkBaseDataAccessor<T> dataAccessor, String path) {
    try {
      List<String> paths = dataAccessor.getChildNames(path, AccessOption.PERSISTENT);
      return paths == null ? Collections.emptyList() : paths;
    } catch (Exception e) {
      LOGGER.error("Error when listing contents in path: {}", path, e);
      throw e;
    }
  }

[gaojieliu]

Can we use VeniceConcurrentHashMap here so that we don't need to recreate a map whenever there is a chance in StoreConfigChangedListener?

[gaojieliu]

We should use ^ as getStoreConfig can update loadedStoreConfigMap concurrently when store configs are missing from the map.