HealthCheckServiceInstanceListSupplier docs say all instances are returned when none are healthy, but code returns an empty list

Đã đóng
  • Có gì không ổn?
#1,771 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Một pull request liên quan đã được merge.

  • #1772 của @bhosale-mahesh — đã merge
  • #1773 của @bhosale-mahesh — đã đóng, không merge

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
50/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
java, spring, spring-boot
Lĩnh vực
backend, documentation, testing

Hướng nghiên cứu

Start by reading HealthCheckServiceInstanceListSupplier.java around the Flux.merge result handling and comparing the behavior with the reference documentation. Check HealthCheckServiceInstanceListSupplierTests.java, especially shouldEmitAllInstancesIfAllIsAliveChecksFailed, to understand the currently asserted empty-list behavior. Before changing anything, determine from project discussion whether the intended fix is documentation, Javadoc, the test name, or the implementation; done means the documented, tested, and implemented behavior agree.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

documentation

Summary

The docs, javadoc and one test name say HealthCheckServiceInstanceListSupplier returns all instances when none are healthy. Code and test assertions return an empty list. Checked on v5.0.3 and main.

What the docs say

Instance Health-Check for LoadBalancer:

It regularly verifies if the instances provided by a delegate ServiceInstanceListSupplier are still alive and only returns the healthy instances, unless there are none - then it returns all the retrieved instances.

Class javadoc says "...only returns the healthy one, unless there are none."

What the code does

When all checks fail, Flux.merge(checks) emits nothing. Both paths then produce an empty list:

HealthCheckServiceInstanceListSupplier.java lines for the result / defaultIfEmpty(result) / collectList() block

List<ServiceInstance> result = new ArrayList<>();
if (healthCheck.isUpdateResultsList()) {
	return Flux.merge(checks).map(alive -> {
		result.add(alive);
		return result;
	}).defaultIfEmpty(result);   // result is empty
}
return Flux.merge(checks).collectList();   // empty list

What the tests assert

shouldEmitAllInstancesIfAllIsAliveChecksFailed: the name says "all", the assertion expects an empty list.

HealthCheckServiceInstanceListSupplierTests.java lines for that test

.expectNext(Lists.list())

shouldRepeatIsAliveChecksIndefinitely and shouldTimeoutIsAliveCheck also expect [] in the first round.

Question

Which one is intended?

  • Docs are wrong: Will create PR for fix in docs, javadoc and the test name.
  • Code is wrong: Will create PR returns all instances when none are healthy, with tests.
Ngôn ngữ chính
Java
Star
751
Fork
748
Merge trung bình
1 ngày 1 giờ
Pull request đã merge (30 ngày)
17

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của spring-cloud/spring-cloud-commons

Tất cả issue của spring-cloud/spring-cloud-commons

Issue tương tự

Thêm issue về Java

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.