You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "hungphan227 (via GitHub)" <gi...@apache.org> on 2023/07/07 02:34:02 UTC
[GitHub] [james-project] hungphan227 opened a new pull request, #1637: James 2600 implement an object storage healthcheck
hungphan227 opened a new pull request, #1637:
URL: https://github.com/apache/james-project/pull/1637
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255176382
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/DockerAwsS3Container.java:
##########
@@ -61,6 +61,21 @@ public void stop() {
awsS3Container.stop();
}
+ public void pause() {
+ awsS3Container.getDockerClient().pauseContainerCmd(awsS3Container.getContainerId()).exec();
+ }
+
+ public void unpause() {
+ awsS3Container.getDockerClient().unpauseContainerCmd(awsS3Container.getContainerId()).exec();
+ }
+
+ public boolean isPaused() {
+ return awsS3Container.getDockerClient().inspectContainerCmd(awsS3Container.getContainerId())
+ .exec()
+ .getState()
+ .getPaused();
Review Comment:
TAB idents should be 4 spaces not 8.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on PR #1637:
URL: https://github.com/apache/james-project/pull/1637#issuecomment-1625007803
I have updated by all comments. Please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1637:
URL: https://github.com/apache/james-project/pull/1637#issuecomment-1627959585
This test is failing `org.apache.james.webadmin.integration.rabbitmq.RabbitMQWebAdminServerIntegrationImmutableTest.healthCheckComponentsList` ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255178437
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3HealthCheckTest.java:
##########
@@ -0,0 +1,64 @@
+package org.apache.james.blob.objectstorage.aws;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.james.blob.api.TestBlobId;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(DockerAwsS3Extension.class)
+public class S3HealthCheckTest {
+
+ private S3HealthCheck s3HealthCheck;
+
+ @BeforeEach
+ void setUp(DockerAwsS3Container dockerAwsS3) {
+ AwsS3AuthConfiguration authConfiguration = AwsS3AuthConfiguration.builder()
+ .endpoint(dockerAwsS3.getEndpoint())
+ .accessKeyId(DockerAwsS3Container.ACCESS_KEY_ID)
+ .secretKey(DockerAwsS3Container.SECRET_ACCESS_KEY)
+ .build();
+
+ S3BlobStoreConfiguration s3Configuration = S3BlobStoreConfiguration.builder()
+ .authConfiguration(authConfiguration)
+ .region(dockerAwsS3.dockerAwsS3().region())
+ .build();
+
+ S3BlobStoreDAO s3BlobStoreDAO = new S3BlobStoreDAO(s3Configuration, new TestBlobId.Factory());
+ s3HealthCheck = new S3HealthCheck(s3BlobStoreDAO);
+ }
+
+ @AfterEach
+ void reset(DockerAwsS3Container dockerAwsS3) {
+ if (dockerAwsS3.isPaused()) {
+ dockerAwsS3.unpause();
+ }
+ }
+
+ @Test
+ void checkShouldReturnHealthyWhenS3IsRunning() {
+ Result check = s3HealthCheck.check().block();
+ assertThat(check.isHealthy()).isTrue();
+ }
+
+ @Test
+ void checkShouldReturnUnhealthyWhenS3IsNotRunning(DockerAwsS3Container dockerAwsS3) {
+ dockerAwsS3.pause();
+ Result check = s3HealthCheck.check().block();
+ assertThat(check.isUnHealthy()).isTrue();
+ }
+
+ @Test
+ void checkShouldDetectWhenCassandraRecovered(DockerAwsS3Container dockerAwsS3) {
Review Comment:
Cassandra?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255177344
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
`ObjectStore` is a better wording IMO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255218870
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
Currently I put object storage health check file in blob-s3 module. However, because this is generic health check for storage object, not s3 alone, I think I should move to it to either blob-api or blob-common. Where do you think it should be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255178003
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public S3HealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ Flux<BucketName> bucketNameFlux = Flux.from(blobStoreDAO.listBuckets())
+ .timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT));
+ return bucketNameFlux.collect(Collectors.toList()).map(ele -> Result.healthy(COMPONENT_NAME))
+ .onErrorResume(e -> Mono.just(Result.unhealthy(COMPONENT_NAME, "Error checking S3", e)));
Review Comment:
```suggestion
public Mono<Result> check() {
Flux<BucketName> bucketNameFlux = Flux.from(blobStoreDAO.listBuckets())
.timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT));
return Flux.from(blobStoreDAO.listBuckets())
.timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT))
.thenReturn(Result.healthy(COMPONENT_NAME))
.onErrorResume(e -> Mono.just(Result.unhealthy(COMPONENT_NAME, "Error checking ObjectSotrage", e)));
```
Is a more efficient reactive style IMO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255176173
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3HealthCheckTest.java:
##########
@@ -0,0 +1,64 @@
+package org.apache.james.blob.objectstorage.aws;
Review Comment:
You are missing the compulsory Apache V2 license.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255218870
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
@chibenwa Currently I put object storage health check file in blob-s3 module. However, because this is generic health check for object storage, not s3 alone, I think I should move to it to either blob-api or blob-common. Where do you think it should be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255436238
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
I moved object storage health check file to blob-s3 module
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255527026
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ObjectStorageHealthCheck.java:
##########
@@ -0,0 +1,59 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import java.time.Duration;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class ObjectStorageHealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 10;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("ObjectStorage");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public ObjectStorageHealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ return Flux.from(blobStoreDAO.listBuckets())
Review Comment:
Q: Is it ok if using `listBuckets` for health check in prod?
I don't find an api for healthcheck in aws s3 sdk.
For Minio supports API for health checks, but it is not generic for all s3 platforms. https://min.io/docs/minio/linux/operations/monitoring/healthcheck-probe.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255202452
##########
server/container/guice/distributed/src/main/java/org/apache/james/modules/blobstore/BlobStoreModulesChooser.java:
##########
@@ -22,13 +22,16 @@
import java.util.List;
import java.util.Optional;
+import org.apache.james.backends.cassandra.utils.CassandraHealthCheck;
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255177053
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public S3HealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ Flux<BucketName> bucketNameFlux = Flux.from(blobStoreDAO.listBuckets())
+ .timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT));
Review Comment:
5 seconds is a rather should timeout to me. I would increse it a bit...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255218870
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
Currently I put object storage health check file in blob-s3 module. However, because this is generic health check for object storage, not s3 alone, I think I should move to it to either blob-api or blob-common. Where do you think it should be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] hungphan227 commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255436238
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
Review Comment:
I moved object storage health check file to blob-api module
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255527026
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ObjectStorageHealthCheck.java:
##########
@@ -0,0 +1,59 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import java.time.Duration;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class ObjectStorageHealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 10;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("ObjectStorage");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public ObjectStorageHealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ return Flux.from(blobStoreDAO.listBuckets())
Review Comment:
Q: Is it ok if using `listBuckets` for health check in prod?
I don't find an api for healthcheck in aws s3 sdk.
For Minio supports API for health checks, but it is not generic for all s3 platforms. https://min.io/docs/minio/linux/operations/monitoring/healthcheck-probe.html
just my concern, it is great if has an answer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1257648945
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ObjectStorageHealthCheck.java:
##########
@@ -0,0 +1,59 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import java.time.Duration;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class ObjectStorageHealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 10;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("ObjectStorage");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public ObjectStorageHealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ return Flux.from(blobStoreDAO.listBuckets())
Review Comment:
listBuckets is a relatively cheap request, think it is ok to use it for a healthcheck
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #1637: James 2600 implement an object storage healthcheck
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #1637:
URL: https://github.com/apache/james-project/pull/1637
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1637: James 2600 implement an object storage healthcheck
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1637:
URL: https://github.com/apache/james-project/pull/1637#discussion_r1255219068
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3HealthCheck.java:
##########
@@ -0,0 +1,61 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws;
+
+import java.time.Duration;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+
+import org.apache.james.blob.api.BlobStoreDAO;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.core.healthcheck.ComponentName;
+import org.apache.james.core.healthcheck.HealthCheck;
+import org.apache.james.core.healthcheck.Result;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+public class S3HealthCheck implements HealthCheck {
+
+ private static final Integer HEALTH_CHECK_TIMEOUT = 5;
+
+ private static final ComponentName COMPONENT_NAME = new ComponentName("S3");
+
+ private final BlobStoreDAO blobStoreDAO;
+
+ @Inject
+ public S3HealthCheck(BlobStoreDAO blobStoreDAO) {
+ this.blobStoreDAO = blobStoreDAO;
+ }
+
+ @Override
+ public ComponentName componentName() {
+ return COMPONENT_NAME;
+ }
+
+ @Override
+ public Mono<Result> check() {
+ Flux<BucketName> bucketNameFlux = Flux.from(blobStoreDAO.listBuckets())
+ .timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT));
+ return bucketNameFlux.collect(Collectors.toList()).map(ele -> Result.healthy(COMPONENT_NAME))
+ .onErrorResume(e -> Mono.just(Result.unhealthy(COMPONENT_NAME, "Error checking S3", e)));
Review Comment:
```suggestion
public Mono<Result> check() {
return Flux.from(blobStoreDAO.listBuckets())
.timeout(Duration.ofSeconds(HEALTH_CHECK_TIMEOUT))
.thenReturn(Result.healthy(COMPONENT_NAME))
.onErrorResume(e -> Mono.just(Result.unhealthy(COMPONENT_NAME, "Error checking ObjectSotrage", e)));
```
is more accurate :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org