[ISSUE #11890] Check instance existence before expiring its metadata #11932
Conversation
| @@ -126,4 +127,16 @@ public String toString() { | |||
| public static String genMetadataId(String ip, int port, String cluster) { | |||
| return ip + InternetAddressUtil.IP_PORT_SPLITER + port + InternetAddressUtil.IP_PORT_SPLITER + cluster; | |||
| } | |||
|
|
|||
| public static String getClusterFromMetadataId(String metadataId) { | |||
There was a problem hiding this comment.
这3个方法建议放置到 Cleaner中，作为私有方法
There was a problem hiding this comment.
这3个方法建议放置到 Cleaner中，作为私有方法
这几个方法只和metadataId的结构有关系，似乎应该放在生成metadataId的地方，并作为公共方法比较好？
There was a problem hiding this comment.
但是只有这里有需要使用， 我觉得还是放cleaner里作为私有方法， 如果以后真需要再改。
There was a problem hiding this comment.
但是只有这里有需要使用， 我觉得还是放cleaner里作为私有方法， 如果以后真需要再改。
问题是如果后面其他地方有需要用，比较难发现某个cleaner已经定义了一个私有方法？
There was a problem hiding this comment.
这个问题你放在这里也可能是一样的， 这个必须通过review才可以，如果你一定要单独处理，要不就放到一个工具类里。
我的意见是暂时没有必要， 如果后续有需要，再做抽象即可， 不要为了不存在的需求做所谓的优化。
There was a problem hiding this comment.
为啥生成metadataID的方法是在这个类，解析metadataID又要放在别的类呢？这个metadataID本来就算是InstancePublishInfo的一个属性。
当我们有解析metadataID的需求的时候，我觉得很自然会去找到生成metadataID的类去看metadataID的结构，所以这3个方法放在这个类也是很合理的。
There was a problem hiding this comment.
因为只有你这个地方要解析， 没有别的地方要解析， 说白了要解析是你这个地方的特殊逻辑，当然要以私有方法方式支持。
There was a problem hiding this comment.
要不然你就把所有metadataID的相关操作，都封装到一个工具类里面。
There was a problem hiding this comment.
说白了， 现在这个PR只是为了修复这个问题的临时手段， 在临时手段中允许暂时的去解析metadataID，获取对应的数据，先修复问题，不考虑性能和处理手段，长期必须要合理处理这个机制，可能根本就不用去解析metadataId。
所以现在就应该把metadataID的解析工作，收敛到这个特殊的修复逻辑中， 不能发散成为一个通用手段。除非以后确定metadataID需要被拆分解析，如果到了那一步， 我反而倾向吧metadataID变为一个Object，而不是简单的字符串。
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
Resolve #11890
Brief changelog
Update ExpiredMetadataCleaner to check instance existence before expiring its metadata
Verifying this change
Follow reproducing steps in #11890 to verify.
Checklist
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.