-
Notifications
You must be signed in to change notification settings - Fork 487
JAMES-4156 Blobstore deleted message vault v2 #2899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Outdated
Show resolved
Hide resolved
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Outdated
Show resolved
Hide resolved
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Show resolved
Hide resolved
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Outdated
Show resolved
Hide resolved
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Outdated
Show resolved
Hide resolved
...x/plugin/deleted-messages-vault/src/main/java/org/apache/james/vault/VaultConfiguration.java
Outdated
Show resolved
Hide resolved
...in/deleted-messages-vault/src/main/java/org/apache/james/vault/blob/BlobIdTimeGenerator.java
Outdated
Show resolved
Hide resolved
quantranhong1999
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read it.
|
Will work on extra tests now |
|
Question that I was asking myself while doing this though: is a V2 necessary? Why not just refactoring the initial BlobStoreDeletedMessageVault? The only thing that changes here is the append (write in new single bucket). The rest is the same logic (it works with the underlying metadata untouched) The rest that is current design specific is for the gc task (the expired deleteExpiredMessages part stuff). Just opening a discussion here. |
Very good remark that will allow to reduce the overall complexity! Reading the vault will indeed be naturally backward compatible. We just need to ensure that "the purge" still cleans up the old bucket. And we would be good to go IMO. |
|
I think it would make the gc task refactoring easier too... Alright will adapt this one to what we just agreed and then will (in an other PR) tackle the GC part refactoring. @chibenwa agreed? |
|
Slightly different approach so I preferred opening a new PR in case, see #2902 |
Still need some tests to make sure V2 can read and delete blobs created with V1