Conversation
There was a problem hiding this comment.
Add necessary annotations
| public static String CACHE_TIMEOUT = "fs.cv.proxy.timeout"; | ||
| public static String CLUSTER_NAME = "fs.cv.proxy.cluster.name"; | ||
|
|
||
| public static String ZK_ENABLE_VALUE = "1"; |
There was a problem hiding this comment.
What's the ZK_ENABLE_VALUE means? Why used string value
There was a problem hiding this comment.
Indicates whether the function is enabled. ZK stores bytes, so using strings is simpler and more direct.
There was a problem hiding this comment.
This is essentially a judgment value, converted to boolean, it seems unnecessary to convert it again?
| } | ||
|
|
||
| if (zkClient != null) { | ||
| zkClient.close(); |
There was a problem hiding this comment.
Catch possible exceptions
| zkServer = conf.get(ZK_SERVER, ""); | ||
| zkPath = conf.get(ZK_PATH, "/curvine/proxy_enable"); | ||
| proxyEnable = conf.getBoolean(CACHE_ENABLE, true); | ||
| proxyTimeout = (int) conf.getTimeDuration(CACHE_TIMEOUT, "60s", TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
Is it possible to throw an exception here?
There was a problem hiding this comment.
If the configuration is wrong, it is reasonable to throw an exception during initialization
There was a problem hiding this comment.
It's easy to encounter problems when throwing exceptions in the constructor
There was a problem hiding this comment.
What should I do if the configuration is wrong?
| try { | ||
| zkClient = CuratorFrameworkFactory.newClient( | ||
| zkServer, | ||
| proxyTimeout, |
There was a problem hiding this comment.
Both parameters are the same, maybe should make them difference
There was a problem hiding this comment.
They should be identical, hopefully failing fast on ZooKeeper exceptions.
| } | ||
|
|
||
| public CurvineFileSystem getFs() { | ||
| if (!enable.get()) { |
There was a problem hiding this comment.
This method needs to be called every time a file is opened, which will result in a large amount of invalid logs.
There was a problem hiding this comment.
You mean，enable is false normally？
There was a problem hiding this comment.
This has nothing to do with the default value, it depends on the user behavior
| private CurvineFileSystem createCurvineFileSystem() throws Exception { | ||
| CompletableFuture<CurvineFileSystem> future = CompletableFuture.supplyAsync(() -> { | ||
| try { | ||
| String scheme = "cv"; |
There was a problem hiding this comment.
Maybe the filesystem prefix scheme could be mutable
There was a problem hiding this comment.
Anyway, the scheme should be a constants variable
3ef1e0d to
f71ba57
Compare
| } | ||
|
|
||
| public CurvineFileSystem getFs() { | ||
| if (!enable.get()) { |
There was a problem hiding this comment.
You mean，enable is false normally？
|
|
||
| private CuratorFramework zkClient; | ||
| private NodeCache nodeCache; | ||
| private final AtomicBoolean enable; |
|
|
||
| public static String ZK_SERVER = "fs.cv.proxy.zk.server"; | ||
| public static String ZK_PATH = "fs.cv.proxy.zk.path"; | ||
| public static String CACHE_ENABLE = "fs.cv.proxy.enable"; |
There was a problem hiding this comment.
CACHE_ ENABLE->CACHE_ENABLED
fs.cv.proxy.enable->fs.cv.proxy.enabled
| public static String CACHE_TIMEOUT = "fs.cv.proxy.timeout"; | ||
| public static String CLUSTER_NAME = "fs.cv.proxy.cluster.name"; | ||
|
|
||
| public static String ZK_ENABLE_VALUE = "1"; |
No description provided.