mirror of
https://github.com/spring-projects/spring-boot.git
synced 2026-09-17 12:09:16 +00:00
Run a single hello command in MongoDB health indicators
The MongoDB health indicators ran the `hello` command against every database returned by `listDatabaseNames()`. For the reactive indicator these commands were issued in parallel via `flatMap`, so a deployment with many databases per connection could open a connection per database on each health check, dramatically increasing connection usage (a regression from the Spring Data based implementation used previously). Since `hello` is a server-level command whose result is independent of the target database, run it only once, against the `admin` database if available or the first returned database otherwise. The `databases` and `maxWireVersion` details are preserved. See gh-50734 Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
This commit is contained in:
committed by
Andy Wilkinson
parent
790f66c37a
commit
019ea06a6a
+15
-10
@@ -17,9 +17,7 @@
|
||||
package org.springframework.boot.mongodb.health;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import com.mongodb.client.MongoClient;
|
||||
import org.bson.Document;
|
||||
@@ -34,10 +32,13 @@ import org.springframework.util.Assert;
|
||||
* MongoDB.
|
||||
*
|
||||
* @author Christian Dupuis
|
||||
* @author Seonwoo Jung
|
||||
* @since 4.0.0
|
||||
*/
|
||||
public class MongoHealthIndicator extends AbstractHealthIndicator {
|
||||
|
||||
private static final String ADMIN_DATABASE = "admin";
|
||||
|
||||
private static final Document HELLO_COMMAND = Document.parse("{ hello: 1 }");
|
||||
|
||||
private final MongoClient mongoClient;
|
||||
@@ -50,15 +51,19 @@ public class MongoHealthIndicator extends AbstractHealthIndicator {
|
||||
|
||||
@Override
|
||||
protected void doHealthCheck(Health.Builder builder) throws Exception {
|
||||
Map<String, Object> details = new LinkedHashMap<>();
|
||||
List<String> databases = new ArrayList<>();
|
||||
details.put("databases", databases);
|
||||
this.mongoClient.listDatabaseNames().forEach((database) -> {
|
||||
Document result = this.mongoClient.getDatabase(database).runCommand(HELLO_COMMAND);
|
||||
databases.add(database);
|
||||
details.putIfAbsent("maxWireVersion", result.getInteger("maxWireVersion"));
|
||||
});
|
||||
builder.up().withDetails(details);
|
||||
this.mongoClient.listDatabaseNames().forEach(databases::add);
|
||||
Document result = this.mongoClient.getDatabase(getDatabaseName(databases)).runCommand(HELLO_COMMAND);
|
||||
builder.up()
|
||||
.withDetail("databases", databases)
|
||||
.withDetail("maxWireVersion", result.getInteger("maxWireVersion"));
|
||||
}
|
||||
|
||||
private static String getDatabaseName(List<String> databases) {
|
||||
if (databases.contains(ADMIN_DATABASE)) {
|
||||
return ADMIN_DATABASE;
|
||||
}
|
||||
return (!databases.isEmpty()) ? databases.get(0) : ADMIN_DATABASE;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+15
-19
@@ -16,10 +16,7 @@
|
||||
|
||||
package org.springframework.boot.mongodb.health;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import com.mongodb.reactivestreams.client.MongoClient;
|
||||
import org.bson.Document;
|
||||
@@ -35,10 +32,13 @@ import org.springframework.util.Assert;
|
||||
* A {@link ReactiveHealthIndicator} for Mongo.
|
||||
*
|
||||
* @author Yulin Qin
|
||||
* @author Seonwoo Jung
|
||||
* @since 4.0.0
|
||||
*/
|
||||
public class MongoReactiveHealthIndicator extends AbstractReactiveHealthIndicator {
|
||||
|
||||
private static final String ADMIN_DATABASE = "admin";
|
||||
|
||||
private static final Document HELLO_COMMAND = Document.parse("{ hello: 1 }");
|
||||
|
||||
private final MongoClient mongoClient;
|
||||
@@ -51,24 +51,20 @@ public class MongoReactiveHealthIndicator extends AbstractReactiveHealthIndicato
|
||||
|
||||
@Override
|
||||
protected Mono<Health> doHealthCheck(Health.Builder builder) {
|
||||
Mono<Map<String, Object>> healthDetails = Flux.from(this.mongoClient.listDatabaseNames())
|
||||
.flatMap((database) -> Mono.from(this.mongoClient.getDatabase(database).runCommand(HELLO_COMMAND))
|
||||
.map((document) -> new HelloResponse(database, document)))
|
||||
.collectList()
|
||||
.map((responses) -> {
|
||||
Map<String, Object> databaseDetails = new LinkedHashMap<>();
|
||||
List<String> databases = new ArrayList<>();
|
||||
databaseDetails.put("databases", databases);
|
||||
for (HelloResponse response : responses) {
|
||||
databases.add(response.database());
|
||||
databaseDetails.putIfAbsent("maxWireVersion", response.document().getInteger("maxWireVersion"));
|
||||
}
|
||||
return databaseDetails;
|
||||
});
|
||||
return healthDetails.map((details) -> builder.up().withDetails(details).build());
|
||||
Mono<List<String>> databases = Flux.from(this.mongoClient.listDatabaseNames()).collectList();
|
||||
return databases.flatMap((databaseNames) -> Mono
|
||||
.from(this.mongoClient.getDatabase(getDatabaseName(databaseNames)).runCommand(HELLO_COMMAND))
|
||||
.map((result) -> builder.up()
|
||||
.withDetail("databases", databaseNames)
|
||||
.withDetail("maxWireVersion", result.getInteger("maxWireVersion"))
|
||||
.build()));
|
||||
}
|
||||
|
||||
private record HelloResponse(String database, Document document) {
|
||||
private static String getDatabaseName(List<String> databases) {
|
||||
if (databases.contains(ADMIN_DATABASE)) {
|
||||
return ADMIN_DATABASE;
|
||||
}
|
||||
return (!databases.isEmpty()) ? databases.get(0) : ADMIN_DATABASE;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+33
-5
@@ -35,12 +35,14 @@ import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.BDDMockito.then;
|
||||
import static org.mockito.BDDMockito.willAnswer;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
|
||||
/**
|
||||
* Tests for {@link MongoHealthIndicator}.
|
||||
*
|
||||
* @author Christian Dupuis
|
||||
* @author Andy Wilkinson
|
||||
* @author Seonwoo Jung
|
||||
*/
|
||||
class MongoHealthIndicatorTests {
|
||||
|
||||
@@ -52,19 +54,45 @@ class MongoHealthIndicatorTests {
|
||||
MongoClient mongoClient = mock(MongoClient.class);
|
||||
MongoIterable<String> databaseNames = mock(MongoIterable.class);
|
||||
willAnswer((invocation) -> {
|
||||
((Consumer<String>) invocation.getArgument(0)).accept("db");
|
||||
((Consumer<String>) invocation.getArgument(0)).accept("test");
|
||||
((Consumer<String>) invocation.getArgument(0)).accept("admin");
|
||||
return null;
|
||||
}).given(databaseNames).forEach(any());
|
||||
given(mongoClient.listDatabaseNames()).willReturn(databaseNames);
|
||||
MongoDatabase mongoDatabase = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("db")).willReturn(mongoDatabase);
|
||||
given(mongoDatabase.runCommand(Document.parse("{ hello: 1 }"))).willReturn(commandResult);
|
||||
MongoDatabase adminDatabase = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("admin")).willReturn(adminDatabase);
|
||||
given(adminDatabase.runCommand(Document.parse("{ hello: 1 }"))).willReturn(commandResult);
|
||||
MongoHealthIndicator healthIndicator = new MongoHealthIndicator(mongoClient);
|
||||
Health health = healthIndicator.health();
|
||||
assertThat(health.getStatus()).isEqualTo(Status.UP);
|
||||
assertThat(health.getDetails()).containsEntry("maxWireVersion", 10);
|
||||
assertThat(health.getDetails()).containsEntry("databases", List.of("db"));
|
||||
assertThat(health.getDetails()).containsEntry("databases", List.of("test", "admin"));
|
||||
then(commandResult).should().getInteger("maxWireVersion");
|
||||
// the hello command must only be run once, never per listed database
|
||||
then(mongoClient).should(never()).getDatabase("test");
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("unchecked")
|
||||
void mongoUsesFirstDatabaseWhenAdminIsNotVisible() {
|
||||
Document commandResult = mock(Document.class);
|
||||
given(commandResult.getInteger("maxWireVersion")).willReturn(10);
|
||||
MongoClient mongoClient = mock(MongoClient.class);
|
||||
MongoIterable<String> databaseNames = mock(MongoIterable.class);
|
||||
willAnswer((invocation) -> {
|
||||
((Consumer<String>) invocation.getArgument(0)).accept("test");
|
||||
return null;
|
||||
}).given(databaseNames).forEach(any());
|
||||
given(mongoClient.listDatabaseNames()).willReturn(databaseNames);
|
||||
MongoDatabase database = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("test")).willReturn(database);
|
||||
given(database.runCommand(Document.parse("{ hello: 1 }"))).willReturn(commandResult);
|
||||
MongoHealthIndicator healthIndicator = new MongoHealthIndicator(mongoClient);
|
||||
Health health = healthIndicator.health();
|
||||
assertThat(health.getStatus()).isEqualTo(Status.UP);
|
||||
assertThat(health.getDetails()).containsEntry("maxWireVersion", 10);
|
||||
assertThat(health.getDetails()).containsEntry("databases", List.of("test"));
|
||||
then(mongoClient).should(never()).getDatabase("admin");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
+36
-6
@@ -24,6 +24,7 @@ import com.mongodb.reactivestreams.client.MongoClient;
|
||||
import com.mongodb.reactivestreams.client.MongoDatabase;
|
||||
import org.bson.Document;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
import reactor.test.StepVerifier;
|
||||
|
||||
@@ -32,23 +33,26 @@ import org.springframework.boot.health.contributor.Status;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.BDDMockito.then;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
|
||||
/**
|
||||
* Tests for {@link MongoReactiveHealthIndicator}.
|
||||
*
|
||||
* @author Yulin Qin
|
||||
* @author Seonwoo Jung
|
||||
*/
|
||||
class MongoReactiveHealthIndicatorTests {
|
||||
|
||||
@Test
|
||||
void mongoIsUp() {
|
||||
MongoClient mongoClient = mock(MongoClient.class);
|
||||
given(mongoClient.listDatabaseNames()).willReturn(Mono.just("db"));
|
||||
MongoDatabase mongoDatabase = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("db")).willReturn(mongoDatabase);
|
||||
given(mongoClient.listDatabaseNames()).willReturn(Flux.just("test", "admin"));
|
||||
MongoDatabase adminDatabase = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("admin")).willReturn(adminDatabase);
|
||||
Document commandResult = mock(Document.class);
|
||||
given(mongoDatabase.runCommand(Document.parse("{ hello: 1 }"))).willReturn(Mono.just(commandResult));
|
||||
given(adminDatabase.runCommand(Document.parse("{ hello: 1 }"))).willReturn(Mono.just(commandResult));
|
||||
given(commandResult.getInteger("maxWireVersion")).willReturn(10);
|
||||
MongoReactiveHealthIndicator mongoReactiveHealthIndicator = new MongoReactiveHealthIndicator(mongoClient);
|
||||
Mono<Health> health = mongoReactiveHealthIndicator.health();
|
||||
@@ -56,14 +60,40 @@ class MongoReactiveHealthIndicatorTests {
|
||||
assertThat(h.getStatus()).isEqualTo(Status.UP);
|
||||
assertThat(h.getDetails()).containsOnlyKeys("maxWireVersion", "databases");
|
||||
assertThat(h.getDetails()).containsEntry("maxWireVersion", 10);
|
||||
assertThat(h.getDetails()).containsEntry("databases", List.of("db"));
|
||||
assertThat(h.getDetails()).containsEntry("databases", List.of("test", "admin"));
|
||||
}).expectComplete().verify(Duration.ofSeconds(30));
|
||||
// the hello command must only be run once, never per listed database
|
||||
then(mongoClient).should(never()).getDatabase("test");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mongoUsesFirstDatabaseWhenAdminIsNotVisible() {
|
||||
MongoClient mongoClient = mock(MongoClient.class);
|
||||
given(mongoClient.listDatabaseNames()).willReturn(Flux.just("test"));
|
||||
MongoDatabase database = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("test")).willReturn(database);
|
||||
Document commandResult = mock(Document.class);
|
||||
given(database.runCommand(Document.parse("{ hello: 1 }"))).willReturn(Mono.just(commandResult));
|
||||
given(commandResult.getInteger("maxWireVersion")).willReturn(10);
|
||||
MongoReactiveHealthIndicator mongoReactiveHealthIndicator = new MongoReactiveHealthIndicator(mongoClient);
|
||||
Mono<Health> health = mongoReactiveHealthIndicator.health();
|
||||
StepVerifier.create(health).consumeNextWith((h) -> {
|
||||
assertThat(h.getStatus()).isEqualTo(Status.UP);
|
||||
assertThat(h.getDetails()).containsOnlyKeys("maxWireVersion", "databases");
|
||||
assertThat(h.getDetails()).containsEntry("maxWireVersion", 10);
|
||||
assertThat(h.getDetails()).containsEntry("databases", List.of("test"));
|
||||
}).expectComplete().verify(Duration.ofSeconds(30));
|
||||
then(mongoClient).should(never()).getDatabase("admin");
|
||||
}
|
||||
|
||||
@Test
|
||||
void mongoIsDown() {
|
||||
MongoClient mongoClient = mock(MongoClient.class);
|
||||
given(mongoClient.listDatabaseNames()).willThrow(new MongoException("Connection failed"));
|
||||
given(mongoClient.listDatabaseNames()).willReturn(Flux.just("admin"));
|
||||
MongoDatabase adminDatabase = mock(MongoDatabase.class);
|
||||
given(mongoClient.getDatabase("admin")).willReturn(adminDatabase);
|
||||
given(adminDatabase.runCommand(Document.parse("{ hello: 1 }")))
|
||||
.willReturn(Mono.error(new MongoException("Connection failed")));
|
||||
MongoReactiveHealthIndicator mongoReactiveHealthIndicator = new MongoReactiveHealthIndicator(mongoClient);
|
||||
Mono<Health> health = mongoReactiveHealthIndicator.health();
|
||||
StepVerifier.create(health).consumeNextWith((h) -> {
|
||||
|
||||
Reference in New Issue
Block a user