[ISSUE#8653]Fix health check bug #8639

hujun-w-2 · 2022-06-27T23:20:15Z

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

for #8653

Brief changelog

1.持久化实例新增change操作
2.在jraft apply操作中校验client是否还存在，如果存在才更新实例

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

codecov-commenter · 2022-06-28T01:35:28Z

Codecov Report

Merging #8639 (9569560) into develop (7aa74b9) will increase coverage by 1.62%.
The diff coverage is 70.83%.

@@              Coverage Diff              @@
##             develop    #8639      +/-   ##
=============================================
+ Coverage      41.57%   43.20%   +1.62%     
- Complexity      4458     4699     +241     
=============================================
  Files            882      904      +22     
  Lines          32127    32502     +375     
  Branches        3721     3751      +30     
=============================================
+ Hits           13357    14042     +685     
+ Misses         17416    17108     -308     
+ Partials        1354     1352       -2

Impacted Files	Coverage Δ
...ice/impl/PersistentClientOperationServiceImpl.java	`55.62% <69.56%> (+2.34%)`	⬆️
...thcheck/v2/PersistentHealthStatusSynchronizer.java	`100.00% <100.00%> (ø)`
...in/java/com/alibaba/nacos/sys/utils/InetUtils.java	`31.03% <0.00%> (-24.14%)`	⬇️
...ba/nacos/naming/core/v2/client/AbstractClient.java	`43.54% <0.00%> (-19.25%)`	⬇️
...nacos/common/remote/client/grpc/GrpcSdkClient.java	`50.00% <0.00%> (-16.67%)`	⬇️
...s/common/remote/client/grpc/GrpcClusterClient.java	`50.00% <0.00%> (-16.67%)`	⬇️
...aba/nacos/naming/core/v2/index/ServiceStorage.java	`51.66% <0.00%> (-15.73%)`	⬇️
...ent/manager/impl/EphemeralIpPortClientManager.java	`54.00% <0.00%> (-14.00%)`	⬇️
...m/alibaba/nacos/core/remote/ConnectionManager.java	`37.86% <0.00%> (-7.99%)`	⬇️
...ava/com/alibaba/nacos/core/cluster/MemberUtil.java	`72.72% <0.00%> (-7.08%)`	⬇️
... and 94 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aa74b9...9569560. Read the comment docs.

KomachiSion · 2022-07-01T02:46:57Z

...java/com/alibaba/nacos/naming/core/v2/service/impl/PersistentClientOperationServiceImpl.java

@@ -180,6 +208,12 @@ public Response onApply(WriteRequest request) {
 }
 }

+ private void onInstanceChange(Service service, Instance instance, String clientId) {
+ if (clientManager.contains(clientId)) {


Whether this judgment is accurate？

The solution here is that if the instance is logged out, the updated instance will not re register the instance. The logged out "apply" will remove the client, and raft can ensure the order of the logged out "apply" and the updated "apply"

Whether this judgment is accurate？

How about one client register two persistent instance, but only remove one instance?

i will fix it

pixystone · 2022-07-14T01:58:55Z

...java/com/alibaba/nacos/naming/core/v2/service/impl/PersistentClientOperationServiceImpl.java

+ request.setInstance(instance);
+ request.setClientId(clientId);
+ final WriteRequest writeRequest = WriteRequest.newBuilder()
+ .setGroup(Constants.NAMING_PERSISTENT_SERVICE_GROUP_V2)


Pls use group() instead.

pixystone · 2022-07-14T02:03:34Z

...java/com/alibaba/nacos/naming/core/v2/service/impl/PersistentClientOperationServiceImpl.java

@@ -170,6 +195,12 @@ public Response onApply(WriteRequest request) {
 case DELETE:
 onInstanceDeregister(instanceRequest.service, instanceRequest.getClientId());
 break;
+ case CHANGE:
+ if (instanceAndServiceExist(instanceRequest)) {


Discussion: would be better an atomic operation?

Discussion: would be better an atomic operation?

The "log" of "jraft" is executed in sequence, and the next log will not be executed before it is executed, so I don't think it's a big problem here.

KomachiSion · 2022-07-19T02:40:34Z

coverage of PersistentClientOperationServiceImpl.java reduced

@xiaochangbai

* Add exception detail stacks when connect to server failed. * [ISSUE#8653]Fix health check bug (#8639) * fix health bug. * fix health bug. * fix health bug. * test update * format code * 注释 * check service exist * check service exist * test fail * user group() * add unit test. * add unit test. * add unit test. * add unit test. * add unit test. * add unit test. * Add comment for config aspect. * [ISSUE#8481]Unified TRACE capacity building (#8805) * [ISSUE#8481]Unified TRACE capacity building * [ISSUE#8481]Unified TRACE capacity building 1. Event type add to traceEvent. 2. The event about instance add instancePort. 3. Add health check type to healthChangeEvent. 4. Other fixup. * Add Generic Type for Simple Cache Map (#8812) * import config file auth fail (#8844) * import config file auth fail * run ci * run ci2 * Feat/calculate ip count when use batchregister (#8858) Feat/calculate ip count when use batchregister * [ISSUE #8880] add a request exception handler (#8881) * [ISSUE #8880] add a request exception handler Close #8880 * [ISSUE #8880] retry ci * Upgrade to 2.1.1 (#8899) * [ISSUE#8910] bug fix calcu ipCount (#8911) * bug fix calcu ipCount * update logic * judge endpoint (#8933) * [ISSUE #8925] fix that the value of hasQueryString is always false (#8927) Close #8925 * [ISSUES#8905]simplify some code (#8908) * refactor: 💡 update console-ui theme (#8951) ✅ Closes: #8950 * Update IoUtils.java (#8948) An IOException has been catch，so throws IOException Is invalid * [ISSUE #8931] Fix bug :BatchInstanceData cannot Serializable question (#8940) * [fix] fix BatchInstanceData cannot Serializable question * fix update serialVersionUID auto generate * [ISSUE #8925] fix that the replaceAll operation is invalid (#8929) Close #8925 * fix #8947. (#8959) * judge endpoint * add some junit test (#8966) * add some junit test * fix some junit * add license * optimize: internationalization of product description content on landing pages (#8956) * Improve code specifications. (#8965) Flip the call to equals method to avoid null pointer exception. * fix LookupFactoryTest ut / ci (#8985) * fix ci * run ci * check cla report * check style * add some auth plugin test (#8988) * add some auth plugin test * ru ci * remove namespaceShowName uniqueness check (#8989) * add unit test for ReflectUtils (#8999) * add unit test for ReflectUtils * reformat code * [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common] (#8997) * [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common] * [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common] * [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common] * add some utils UT (#9006) add some utils UT * Fix grpc push data not filter by cluster. (#9019) * [#344] rename initialization sql files (#8969) * [ISSUE #9014] performance optimization--refacte TpsMonitorPoint (#9021) * performance optimization * fix check style * fix test unit * fix test unit for `getTimeFormatOfSecond` method. * fix unit test for LimiterTest. * fix error usage case for asserEquals method；eliminate interactions between test methods * Fix spelling mistakes. (#9026) * [ISSUE #9013] enhance grpc client (#9017) * fix issue #9013 * reformat code style * reformat code style * reformat code style * reformat code style * reformat code style * reformat code style * reformat code style * change config name * do some enhance * add queue size to configProperties * reformat code style * Change configuration load order * Fix ServerListMgr is not shutdown. (#9048) * set server.error.include-message to ALWAYS (#9045) * [ISSUE #8934] add header back after convert request to jsonBytes (#8935) * [ISSUE #9023] fix corner case config dataId 'cipher-' that cannot create (#9024) * [ISSUE #9023] fix corner case config dataId 'cipher-' that cannot create * [ISSUE #9023] remove unused import * [ISSUE #9023] PMD fix * [ISSUE #9023] add comments * [ISSUE #9023] should return first token instead of any * [ISSUE #9023] add more EncryptionHandler ut * [ISSUE #9023] fix style * [ISSUE #9023] fix style, keep indentation in blank line Co-authored-by: KomachiSion <[email protected]> Co-authored-by: 胡俊 <[email protected]> Co-authored-by: MajorHe1 <[email protected]> Co-authored-by: GuiShao <[email protected]> Co-authored-by: ZhangShenao <[email protected]> Co-authored-by: chenhao26 <[email protected]> Co-authored-by: onewe <[email protected]> Co-authored-by: 胡俊 <[email protected]> Co-authored-by: liushaoyu-soft <[email protected]> Co-authored-by: heimanba <[email protected]> Co-authored-by: 无处不在 <[email protected]> Co-authored-by: Weizhan Yun <[email protected]> Co-authored-by: liaochuntao <[email protected]> Co-authored-by: @xiaochangbai <[email protected]> Co-authored-by: shiherua <[email protected]> Co-authored-by: Karson <[email protected]> Co-authored-by: yjqg6666 <[email protected]> Co-authored-by: 阿魁 <[email protected]> Co-authored-by: wuwen <[email protected]> Co-authored-by: hutt <[email protected]>

hujun-w-2 and others added 4 commits June 27, 2022 23:55

fix health bug.

0954476

fix health bug.

136ff95

fix health bug.

f613726

test update

637e599

hujun-w-2 closed this Jun 28, 2022

format code

b74350c

hujun-w-2 reopened this Jun 29, 2022

hujun-w-2 requested review from chuntaojun and li-xiao-shuang June 29, 2022 09:33

hujun-w-2 changed the title ~~Fix health bug~~ [#ISSUE#8653]Fix health bug Jun 29, 2022

hujun-w-2 changed the title ~~[#ISSUE#8653]Fix health bug~~ [ISSUE#8653]Fix health bug Jun 29, 2022

Merge branch 'alibaba:develop' into fix_health_bug

e87eb07

hujun-w-2 changed the title ~~[ISSUE#8653]Fix health bug~~ [ISSUE#8653]Fix health check bug Jun 29, 2022

hujun-w-2 added 2 commits June 29, 2022 17:39

注释

e1416c6

Merge remote-tracking branch 'origin/fix_health_bug' into fix_health_bug

4bf99f9

hujun-w-2 requested a review from KomachiSion June 30, 2022 09:15

KomachiSion reviewed Jul 1, 2022

View reviewed changes

hujun-w-2 added 3 commits July 7, 2022 12:41

check service exist

206b5bb

check service exist

e3a52d4

test fail

27ede3d

hujun-w-2 requested a review from KomachiSion July 7, 2022 06:02

pixystone suggested changes Jul 14, 2022

View reviewed changes

user group()

d116bdd

hujun-w-2 added 5 commits July 24, 2022 09:17

add unit test.

c57ecf4

add unit test.

44bbee9

add unit test.

89077e5

add unit test.

d14075b

add unit test.

ddb6736

add unit test.

9569560

KomachiSion approved these changes Jul 25, 2022

View reviewed changes

KomachiSion approved these changes Jul 26, 2022

View reviewed changes

KomachiSion merged commit e950be1 into alibaba:develop Jul 26, 2022

KomachiSion added the kind/bug Category issues or prs related to bug. label Jul 26, 2022

KomachiSion added this to the 2.1.1 milestone Jul 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE#8653]Fix health check bug #8639

[ISSUE#8653]Fix health check bug #8639

hujun-w-2 commented Jun 27, 2022 •

edited

Loading

codecov-commenter commented Jun 28, 2022 •

edited

Loading

KomachiSion Jul 1, 2022

hujun-w-2 Jul 1, 2022

KomachiSion Jul 7, 2022

hujun-w-2 Jul 7, 2022

pixystone Jul 14, 2022

hujun-w-2 Jul 14, 2022

pixystone Jul 14, 2022

hujun-w-2 Jul 14, 2022

KomachiSion commented Jul 19, 2022

[ISSUE#8653]Fix health check bug #8639

[ISSUE#8653]Fix health check bug #8639

Conversation

hujun-w-2 commented Jun 27, 2022 • edited Loading

What is the purpose of the change

Brief changelog

Verifying this change

codecov-commenter commented Jun 28, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KomachiSion commented Jul 19, 2022

hujun-w-2 commented Jun 27, 2022 •

edited

Loading

codecov-commenter commented Jun 28, 2022 •

edited

Loading