Skip to content

Commit e8d9b6f

Browse files
philwebbphilwebb
committed
Polish "Fallback to ping if Solr URL references core"
See gh-16477
1 parent b9764e8 commit e8d9b6f

File tree

2 files changed

+110
-93
lines changed

2 files changed

+110
-93
lines changed

‎spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/solr/SolrHealthIndicator.java

Lines changed: 74 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -16,99 +16,120 @@
1616

1717
package org.springframework.boot.actuate.solr;
1818

19-
import java.io.IOException;
20-
2119
import org.apache.solr.client.solrj.SolrClient;
22-
import org.apache.solr.client.solrj.SolrServerException;
23-
import org.apache.solr.client.solrj.impl.HttpSolrClient;
20+
import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException;
2421
import org.apache.solr.client.solrj.request.CoreAdminRequest;
25-
import org.apache.solr.client.solrj.response.CoreAdminResponse;
2622
import org.apache.solr.common.params.CoreAdminParams;
2723

2824
import org.springframework.boot.actuate.health.AbstractHealthIndicator;
2925
import org.springframework.boot.actuate.health.Health;
3026
import org.springframework.boot.actuate.health.HealthIndicator;
3127
import org.springframework.boot.actuate.health.Status;
32-
import org.springframework.http.HttpStatus;
3328

3429
/**
3530
* {@link HealthIndicator} for Apache Solr.
3631
*
3732
* @author Andy Wilkinson
3833
* @author Stephane Nicoll
3934
* @author Markus Schuch
35+
* @author Phillip Webb
4036
* @since 2.0.0
4137
*/
4238
public class SolrHealthIndicator extends AbstractHealthIndicator {
4339

40+
private static final int HTTP_NOT_FOUND_STATUS = 404;
41+
4442
private final SolrClient solrClient;
4543

46-
private PathType detectedPathType = PathType.UNKNOWN;
44+
private volatile StatusCheck statusCheck;
4745

4846
public SolrHealthIndicator(SolrClient solrClient) {
4947
super("Solr health check failed");
5048
this.solrClient = solrClient;
5149
}
5250

51+
@Override
5352
protected void doHealthCheck(Health.Builder builder) throws Exception {
54-
int statusCode;
53+
int statusCode = initializeStatusCheck();
54+
Status status = (statusCode != 0) ? Status.DOWN : Status.UP;
55+
builder.status(status).withDetail("status", statusCode).withDetail("detectedPathType",
56+
this.statusCheck.getPathType());
57+
}
5558

56-
if (this.detectedPathType == PathType.ROOT) {
57-
statusCode = doCoreAdminCheck();
59+
private int initializeStatusCheck() throws Exception {
60+
StatusCheck statusCheck = this.statusCheck;
61+
if (statusCheck != null) {
62+
// Already initilized
63+
return statusCheck.getStatus(this.solrClient);
5864
}
59-
else if (this.detectedPathType == PathType.PARTICULAR_CORE) {
60-
statusCode = doPingCheck();
65+
try {
66+
return initializeStatusCheck(new RootStatusCheck());
6167
}
62-
else {
63-
// We do not know yet, which is the promising
64-
// health check strategy, so we start trying with
65-
// a CoreAdmin check, which is the common case
66-
try {
67-
statusCode = doCoreAdminCheck();
68-
// When the CoreAdmin request returns with a
69-
// valid response, we can assume that this
70-
// SolrClient is configured with a baseUrl
71-
// pointing to the root path of the Solr instance
72-
this.detectedPathType = PathType.ROOT;
73-
}
74-
catch (HttpSolrClient.RemoteSolrException ex) {
75-
// CoreAdmin requests to not work with
76-
// SolrClients configured with a baseUrl pointing to
77-
// a particular core and a 404 response indicates
78-
// that this might be the case.
79-
if (ex.code() == HttpStatus.NOT_FOUND.value()) {
80-
statusCode = doPingCheck();
81-
// When the SolrPing returns with a valid
82-
// response, we can assume that the baseUrl
83-
// of this SolrClient points to a particular core
84-
this.detectedPathType = PathType.PARTICULAR_CORE;
85-
}
86-
else {
87-
// Rethrow every other response code leaving us
88-
// in the dark about the type of the baseUrl
89-
throw ex;
90-
}
68+
catch (RemoteSolrException ex) {
69+
// 404 is thrown when SolrClient has a baseUrl pointing to a particular core.
70+
if (ex.code() == HTTP_NOT_FOUND_STATUS) {
71+
return initializeStatusCheck(new ParticularCoreStatusCheck());
9172
}
73+
throw ex;
9274
}
93-
Status status = (statusCode != 0) ? Status.DOWN : Status.UP;
94-
builder.status(status).withDetail("status", statusCode).withDetail("detectedPathType",
95-
this.detectedPathType.toString());
9675
}
9776

98-
private int doCoreAdminCheck() throws IOException, SolrServerException {
99-
CoreAdminRequest request = new CoreAdminRequest();
100-
request.setAction(CoreAdminParams.CoreAdminAction.STATUS);
101-
CoreAdminResponse response = request.process(this.solrClient);
102-
return response.getStatus();
77+
private int initializeStatusCheck(StatusCheck statusCheck) throws Exception {
78+
int result = statusCheck.getStatus(this.solrClient);
79+
this.statusCheck = statusCheck;
80+
return result;
10381
}
10482

105-
private int doPingCheck() throws IOException, SolrServerException {
106-
return this.solrClient.ping().getStatus();
83+
/**
84+
* Strategy used to perform the status check.
85+
*/
86+
private abstract static class StatusCheck {
87+
88+
private final String pathType;
89+
90+
StatusCheck(String pathType) {
91+
this.pathType = pathType;
92+
}
93+
94+
abstract int getStatus(SolrClient client) throws Exception;
95+
96+
String getPathType() {
97+
return this.pathType;
98+
}
99+
100+
}
101+
102+
/**
103+
* {@link StatusCheck} used when {@code baseUrl} points to the root context.
104+
*/
105+
private static class RootStatusCheck extends StatusCheck {
106+
107+
RootStatusCheck() {
108+
super("root");
109+
}
110+
111+
@Override
112+
public int getStatus(SolrClient client) throws Exception {
113+
CoreAdminRequest request = new CoreAdminRequest();
114+
request.setAction(CoreAdminParams.CoreAdminAction.STATUS);
115+
return request.process(client).getStatus();
116+
}
117+
107118
}
108119

109-
enum PathType {
120+
/**
121+
* {@link StatusCheck} used when {@code baseUrl} points to the particular core.
122+
*/
123+
private static class ParticularCoreStatusCheck extends StatusCheck {
110124

111-
ROOT, PARTICULAR_CORE, UNKNOWN
125+
ParticularCoreStatusCheck() {
126+
super("particular core");
127+
}
128+
129+
@Override
130+
public int getStatus(SolrClient client) throws Exception {
131+
return client.ping().getStatus();
132+
}
112133

113134
}
114135

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/solr/SolrHealthIndicatorTests.java

Lines changed: 36 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import java.io.IOException;
2020

2121
import org.apache.solr.client.solrj.SolrClient;
22-
import org.apache.solr.client.solrj.impl.HttpSolrClient;
22+
import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException;
2323
import org.apache.solr.client.solrj.request.CoreAdminRequest;
2424
import org.apache.solr.client.solrj.response.SolrPingResponse;
2525
import org.apache.solr.common.util.NamedList;
@@ -43,6 +43,8 @@
4343
* Tests for {@link SolrHealthIndicator}
4444
*
4545
* @author Andy Wilkinson
46+
* @author Markus Schuch
47+
* @author Phillip Webb
4648
*/
4749
public class SolrHealthIndicatorTests {
4850

@@ -56,94 +58,88 @@ public void close() {
5658
}
5759

5860
@Test
59-
public void solrIsUpWithBaseUrlPointsToRoot() throws Exception {
61+
public void healthWhenSolrStatusUpAndBaseUrlPointsToRootReturnsUp() throws Exception {
6062
SolrClient solrClient = mock(SolrClient.class);
6163
given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(0));
6264
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
63-
Health health = healthIndicator.health();
64-
assertThat(health.getStatus()).isEqualTo(Status.UP);
65-
assertThat(health.getDetails().get("status")).isEqualTo(0);
66-
assertThat(health.getDetails().get("detectedPathType")).isEqualTo(SolrHealthIndicator.PathType.ROOT.toString());
65+
assertHealth(healthIndicator, Status.UP, 0, "root");
6766
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
6867
verifyNoMoreInteractions(solrClient);
6968
}
7069

7170
@Test
72-
public void solrIsUpWithBaseUrlPointsToParticularCore() throws Exception {
71+
public void healthWhenSolrStatusDownAndBaseUrlPointsToRootReturnsDown() throws Exception {
7372
SolrClient solrClient = mock(SolrClient.class);
74-
given(solrClient.request(any(CoreAdminRequest.class), isNull()))
75-
.willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null));
76-
given(solrClient.ping()).willReturn(mockPingResponse(0));
73+
given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(400));
7774
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
78-
Health health = healthIndicator.health();
79-
assertThat(health.getStatus()).isEqualTo(Status.UP);
80-
assertThat(health.getDetails().get("status")).isEqualTo(0);
81-
assertThat(health.getDetails().get("detectedPathType"))
82-
.isEqualTo(SolrHealthIndicator.PathType.PARTICULAR_CORE.toString());
75+
assertHealth(healthIndicator, Status.DOWN, 400, "root");
8376
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
84-
verify(solrClient, times(1)).ping();
8577
verifyNoMoreInteractions(solrClient);
8678
}
8779

8880
@Test
89-
public void pathTypeIsRememberedForConsecutiveChecks() throws Exception {
81+
public void healthWhenSolrStatusUpAndBaseUrlPointsToParticularCoreReturnsUp() throws Exception {
9082
SolrClient solrClient = mock(SolrClient.class);
9183
given(solrClient.request(any(CoreAdminRequest.class), isNull()))
92-
.willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null));
84+
.willThrow(new RemoteSolrException("mock", 404, "", null));
9385
given(solrClient.ping()).willReturn(mockPingResponse(0));
9486
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
95-
healthIndicator.health();
87+
assertHealth(healthIndicator, Status.UP, 0, "particular core");
9688
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
9789
verify(solrClient, times(1)).ping();
9890
verifyNoMoreInteractions(solrClient);
99-
healthIndicator.health();
100-
verify(solrClient, times(2)).ping();
101-
verifyNoMoreInteractions(solrClient);
10291
}
10392

10493
@Test
105-
public void solrIsUpAndRequestFailedWithBaseUrlPointsToRoot() throws Exception {
94+
public void healthWhenSolrStatusDownAndBaseUrlPointsToParticularCoreReturnsDown() throws Exception {
10695
SolrClient solrClient = mock(SolrClient.class);
107-
given(solrClient.request(any(CoreAdminRequest.class), isNull())).willReturn(mockResponse(400));
96+
given(solrClient.request(any(CoreAdminRequest.class), isNull()))
97+
.willThrow(new RemoteSolrException("mock", 404, "", null));
98+
given(solrClient.ping()).willReturn(mockPingResponse(400));
10899
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
109-
Health health = healthIndicator.health();
110-
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
111-
assertThat(health.getDetails().get("status")).isEqualTo(400);
112-
assertThat(health.getDetails().get("detectedPathType")).isEqualTo(SolrHealthIndicator.PathType.ROOT.toString());
100+
assertHealth(healthIndicator, Status.DOWN, 400, "particular core");
113101
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
102+
verify(solrClient, times(1)).ping();
114103
verifyNoMoreInteractions(solrClient);
115104
}
116105

117106
@Test
118-
public void solrIsUpAndRequestFailedWithBaseUrlPointsToParticularCore() throws Exception {
107+
public void healthWhenSolrConnectionFailsReturnsDown() throws Exception {
119108
SolrClient solrClient = mock(SolrClient.class);
120109
given(solrClient.request(any(CoreAdminRequest.class), isNull()))
121-
.willThrow(new HttpSolrClient.RemoteSolrException("mock", 404, "", null));
122-
given(solrClient.ping()).willReturn(mockPingResponse(400));
110+
.willThrow(new IOException("Connection failed"));
123111
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
124112
Health health = healthIndicator.health();
125113
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
126-
assertThat(health.getDetails().get("status")).isEqualTo(400);
127-
assertThat(health.getDetails().get("detectedPathType"))
128-
.isEqualTo(SolrHealthIndicator.PathType.PARTICULAR_CORE.toString());
114+
assertThat((String) health.getDetails().get("error")).contains("Connection failed");
129115
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
130-
verify(solrClient, times(1)).ping();
131116
verifyNoMoreInteractions(solrClient);
132117
}
133118

134119
@Test
135-
public void solrIsDown() throws Exception {
120+
public void healthWhenMakingMultipleCallsRemembersStatusStrategy() throws Exception {
136121
SolrClient solrClient = mock(SolrClient.class);
137122
given(solrClient.request(any(CoreAdminRequest.class), isNull()))
138-
.willThrow(new IOException("Connection failed"));
123+
.willThrow(new RemoteSolrException("mock", 404, "", null));
124+
given(solrClient.ping()).willReturn(mockPingResponse(0));
139125
SolrHealthIndicator healthIndicator = new SolrHealthIndicator(solrClient);
140-
Health health = healthIndicator.health();
141-
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
142-
assertThat((String) health.getDetails().get("error")).contains("Connection failed");
126+
healthIndicator.health();
143127
verify(solrClient, times(1)).request(any(CoreAdminRequest.class), isNull());
128+
verify(solrClient, times(1)).ping();
129+
verifyNoMoreInteractions(solrClient);
130+
healthIndicator.health();
131+
verify(solrClient, times(2)).ping();
144132
verifyNoMoreInteractions(solrClient);
145133
}
146134

135+
private void assertHealth(SolrHealthIndicator healthIndicator, Status expectedStatus, int expectedStatusCode,
136+
String expectedPathType) {
137+
Health health = healthIndicator.health();
138+
assertThat(health.getStatus()).isEqualTo(expectedStatus);
139+
assertThat(health.getDetails().get("status")).isEqualTo(expectedStatusCode);
140+
assertThat(health.getDetails().get("detectedPathType")).isEqualTo(expectedPathType);
141+
}
142+
147143
private NamedList<Object> mockResponse(int status) {
148144
NamedList<Object> response = new NamedList<>();
149145
NamedList<Object> headers = new NamedList<>();

0 commit comments

Comments
 (0)