[GitHub] [james-project] Arsnael commented on a change in pull request #436: JAMES-3586 CL one option for the Cassandra blob store

[GitBox <gi...@apache.org>]

Arsnael commented on a change in pull request #436:
URL: https://github.com/apache/james-project/pull/436#discussion_r634214463



##########
File path: server/blob/blob-cassandra/pom.xml
##########
@@ -65,6 +65,10 @@
             <groupId>${james.groupId}</groupId>
             <artifactId>james-server-util</artifactId>
         </dependency>
+        <dependency>
+            <groupId>${james.groupId}</groupId>
+            <artifactId>metrics-logger</artifactId>

Review comment:
       I don't like that... at all... but I had to add something for CassandraBlobStoreFactory that is not in the test package and is being used by other test classes in other modules... Not sure what would be the best way here... or add the metrics as a param in forTesting method? 

##########
File path: server/blob/blob-cassandra/src/main/java/org/apache/james/blob/cassandra/CassandraBlobStoreDAO.java
##########
@@ -166,6 +166,23 @@ private boolean isDefaultBucket(BucketName bucketName) {
     }
 
     private Mono<ByteBuffer> readPart(BucketName bucketName, BlobId blobId, Integer partIndex) {
+        if (configuration.isOptimisticConsistencyLevel()) {
+            return readPartClOne(bucketName, blobId, partIndex)

Review comment:
       Was not sure if we need to apply it on the read for readPart too? But my guess is yes? Just want to be sure :)

##########
File path: server/blob/blob-cassandra/src/main/java/org/apache/james/blob/cassandra/CassandraBlobStoreDAO.java
##########
@@ -168,7 +182,9 @@ private boolean isDefaultBucket(BucketName bucketName) {
     private Mono<ByteBuffer> readPart(BucketName bucketName, BlobId blobId, Integer partIndex) {
         if (configuration.isOptimisticConsistencyLevel()) {
             return readPartClOne(bucketName, blobId, partIndex)
-                .switchIfEmpty(readPartClDefault(bucketName, blobId, partIndex));
+                .doOnNext(any -> metricClOneHitCount.increment())

Review comment:
       Should we use the same metric for hit and miss between readBlob and readPart?




-- 
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.

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