You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/06/15 08:55:25 UTC
[GitHub] [james-project] chibenwa opened a new pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
chibenwa opened a new pull request #498:
URL: https://github.com/apache/james-project/pull/498
--
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
[GitHub] [james-project] Arsnael commented on a change in pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #498:
URL: https://github.com/apache/james-project/pull/498#discussion_r654294090
##########
File path: server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/cassandra/CassandraMailQueueBrowser.java
##########
@@ -131,6 +131,14 @@ public CassandraMailQueueItemView next() {
.flatMapSequential(slice -> browseSlice(queueName, slice));
}
+ Flux<EnqueuedItemWithSlicingContext> browseReferences(MailQueueName queueName, Instant start, Instant end) {
+ return allSlicesStartingAt(start)
+ .doOnNext(n -> System.out.println(" --- " + n + " slice looked up"))
Review comment:
I fixed it, but I do have a concern as I had my hands directly in the code. This newly created method does not seem to be used anywhere. So did you forget to switch to it from pieces of code using the other `browseReferences` method? Or is this useless and should be removed?
##########
File path: server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/cassandra/CassandraMailQueueBrowser.java
##########
@@ -131,6 +131,14 @@ public CassandraMailQueueItemView next() {
.flatMapSequential(slice -> browseSlice(queueName, slice));
}
+ Flux<EnqueuedItemWithSlicingContext> browseReferences(MailQueueName queueName, Instant start, Instant end) {
+ return allSlicesStartingAt(start)
+ .doOnNext(n -> System.out.println(" --- " + n + " slice looked up"))
Review comment:
cc @chibenwa
--
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
[GitHub] [james-project] Arsnael commented on pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #498:
URL: https://github.com/apache/james-project/pull/498#issuecomment-864714656
Squashed fixups
--
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
[GitHub] [james-project] Arsnael commented on pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #498:
URL: https://github.com/apache/james-project/pull/498#issuecomment-862036553
Rebased and squash fixups
--
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
[GitHub] [james-project] Arsnael commented on a change in pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #498:
URL: https://github.com/apache/james-project/pull/498#discussion_r651619305
##########
File path: server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/cassandra/CassandraMailQueueViewModule.java
##########
@@ -120,6 +127,15 @@
.addPartitionKey(BrowseStartTable.QUEUE_NAME, text())
.addColumn(BrowseStartTable.BROWSE_START, timestamp()))
+ .table(ContentStartTable.TABLE_NAME)
+ .comment("this table allows to find the starting point of content from the table: "
+ + EnqueuedMailsTable.TABLE_NAME + " in order to make a browse operations through mail queues. Strictly " +
Review comment:
```suggestion
+ EnqueuedMailsTable.TABLE_NAME + " in order to make a browse operation through mail queues. Strictly " +
```
##########
File path: server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/cassandra/CassandraMailQueueViewModule.java
##########
@@ -120,6 +127,15 @@
.addPartitionKey(BrowseStartTable.QUEUE_NAME, text())
.addColumn(BrowseStartTable.BROWSE_START, timestamp()))
+ .table(ContentStartTable.TABLE_NAME)
+ .comment("this table allows to find the starting point of content from the table: "
+ + EnqueuedMailsTable.TABLE_NAME + " in order to make a browse operations through mail queues. Strictly " +
+ "before browse start, it enables aueue cleanup.")
Review comment:
```suggestion
"before browse start, it enables queue cleanup.")
```
##########
File path: server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/RabbitMQMailQueueTest.java
##########
@@ -288,6 +292,51 @@ void browseAndDequeueShouldCombineWellWhenDifferentSlices() throws Exception {
.containsExactly("3-4", "3-5", "5-1", "5-2", "5-3", "5-4", "5-5");
}
+ @Test
+ void enqueuedEmailsShouldEventuallyBeCleaned() {
+ ManageableMailQueue mailQueue = getManageableMailQueue();
+ int emailCount = 5;
+
+ clock.setInstant(IN_SLICE_1);
+ enqueueSomeMails(namePatternForSlice(1), emailCount);
+
+ clock.setInstant(IN_SLICE_2);
+ enqueueSomeMails(namePatternForSlice(2), emailCount);
+
+ clock.setInstant(IN_SLICE_3);
+ enqueueSomeMails(namePatternForSlice(3), emailCount);
+
+ clock.setInstant(IN_SLICE_5);
+ enqueueSomeMails(namePatternForSlice(5), emailCount);
+
+ clock.setInstant(IN_SLICE_7);
+ dequeueMails(5);
+ dequeueMails(5);
+ dequeueMails(5);
+ dequeueMails(5);
+
+ // ensure slice 1 was cleaned
+ EnqueuedMailsDAO mailsDAO = new EnqueuedMailsDAO(cassandraCluster.getCassandraCluster().getConf(), new HashBlobId.Factory());
+ MailQueueName queueName = MailQueueName.fromString(mailQueue.getName().asString());
+ Slice slice = Slice.of(Instant.parse("2007-12-03T10:15:00.00Z")).allSlicesTill(IN_SLICE_2, ONE_HOUR_SLICE_WINDOW).findFirst()
Review comment:
```suggestion
Slice slice = Slice.of(IN_SLICE_1).allSlicesTill(IN_SLICE_2, ONE_HOUR_SLICE_WINDOW).findFirst()
```
##########
File path: server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/view/cassandra/EnqueuedMailsDaoTest.java
##########
@@ -148,4 +148,43 @@ void selectEnqueuedMailsShouldWork() throws Exception {
});
});
}
+
+ @Test
+ void selectShouldNotRemoveDeletedSlice() throws Exception {
Review comment:
I don't think the test name is correct here
--
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
[GitHub] [james-project] chibenwa edited a comment on pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
chibenwa edited a comment on pull request #498:
URL: https://github.com/apache/james-project/pull/498#issuecomment-861484323
Blobs are already deleted at dequeue time...
We do not need to delete them in the bkackground.
--
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
[GitHub] [james-project] chibenwa merged pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #498:
URL: https://github.com/apache/james-project/pull/498
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #498:
URL: https://github.com/apache/james-project/pull/498#discussion_r651773911
##########
File path: server/queue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/RabbitMQMailQueueTest.java
##########
@@ -288,6 +292,51 @@ void browseAndDequeueShouldCombineWellWhenDifferentSlices() throws Exception {
.containsExactly("3-4", "3-5", "5-1", "5-2", "5-3", "5-4", "5-5");
}
+ @Test
+ void enqueuedEmailsShouldEventuallyBeCleaned() {
+ ManageableMailQueue mailQueue = getManageableMailQueue();
+ int emailCount = 5;
+
+ clock.setInstant(IN_SLICE_1);
+ enqueueSomeMails(namePatternForSlice(1), emailCount);
+
+ clock.setInstant(IN_SLICE_2);
+ enqueueSomeMails(namePatternForSlice(2), emailCount);
+
+ clock.setInstant(IN_SLICE_3);
+ enqueueSomeMails(namePatternForSlice(3), emailCount);
+
+ clock.setInstant(IN_SLICE_5);
+ enqueueSomeMails(namePatternForSlice(5), emailCount);
+
+ clock.setInstant(IN_SLICE_7);
+ dequeueMails(5);
+ dequeueMails(5);
+ dequeueMails(5);
+ dequeueMails(5);
+
+ // ensure slice 1 was cleaned
+ EnqueuedMailsDAO mailsDAO = new EnqueuedMailsDAO(cassandraCluster.getCassandraCluster().getConf(), new HashBlobId.Factory());
+ MailQueueName queueName = MailQueueName.fromString(mailQueue.getName().asString());
+ Slice slice = Slice.of(Instant.parse("2007-12-03T10:15:00.00Z")).allSlicesTill(IN_SLICE_2, ONE_HOUR_SLICE_WINDOW).findFirst()
Review comment:
Not really, the slice need to be the exact instant
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #498:
URL: https://github.com/apache/james-project/pull/498#discussion_r654362935
##########
File path: server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/view/cassandra/CassandraMailQueueBrowser.java
##########
@@ -131,6 +131,14 @@ public CassandraMailQueueItemView next() {
.flatMapSequential(slice -> browseSlice(queueName, slice));
}
+ Flux<EnqueuedItemWithSlicingContext> browseReferences(MailQueueName queueName, Instant start, Instant end) {
+ return allSlicesStartingAt(start)
+ .doOnNext(n -> System.out.println(" --- " + n + " slice looked up"))
Review comment:
True I moved away from it.
We can remove it.
Rationnals: as items prior browse start are all marked as deleted, then browse is not returning them. I had to use another strategy.
Please @Arsnael can you remove it?
--
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
[GitHub] [james-project] chibenwa commented on pull request #498: JAMES-3601 Piggy back content cleaning upon browse start update
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #498:
URL: https://github.com/apache/james-project/pull/498#issuecomment-861484323
Blobs are already deleted at dequeue time...
--
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