Persist and republish mailbox messages #5072
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awesome. Yes please |
chimp1984
force-pushed
the
persist-and-republish-mailbox-messages
branch
2 times, most recently
from
b63d632 to
b234073
Compare
chimp1984
added a commit
to chimp1984/bisq
that referenced
this pull request
Jan 12, 2021
chimp1984
force-pushed
the
persist-and-republish-mailbox-messages
branch
from
4af829e to
fe44cee
Compare
sqrrm
reviewed
Jan 12, 2021
There was a problem hiding this comment.
Would it make sense to have seednodes request all mailbox messages on startup and then republish the discrepancy between the response and their locally persisted messages?
That would reduce the amount of republishing for the price of only seednodes asking for all messages from the other seednodes on startup.
p2p/src/main/java/bisq/network/p2p/storage/RemovedPayloadsStorageService.java
Outdated
Show resolved
Hide resolved
…sageService instead
…geService instead
…sageService instead
Remove Objects::nonNull check Split handleMailboxItem into 2 methods for persisting and for processing my mailbox msg
…arrives Add republishMailBoxMessages method
We add the date when we add the hash so that we can remove expired data.
We have added the capability in 1.4.0 and have enforced 1.5.1 so no traders can use that old version anymore so the capability check is not needed anymore.
Adjust param
Remove outdated comment Add checkNotNull for senderNodeAddress
Extend TradeMailboxMessage where we had `extends TradeMessage implements MailboxMessage`
For all MailboxMessage we want to be sure they have an expire date. Add getTTL methods and TTL value. We use 7 days for ChatMessages and 30 days for PrivateNotificationMessages. All others use default 15 days. We prefer to keep them explicit and not use a default method in ExpirablePayload or MailboxMessage. The value in PrefixedSealedAndSignedMessage will not be used as the sender use the TTL of the payload and pass that to the MailboxStoragePayload where we store it in the extraMap. That way we can use different TTL even the payload message is encrypted and we would not be able to look it up.
onBundleOfEnvelopes method. In case we received a SendersNodeAddressMessage inside a BundleOfEnvelope we did not set the peers address.
Add size limit. Add log for number of persisted mailbox msg per day
|
Awesome |
sqrrm
reviewed
Jan 14, 2021
p2p/src/main/java/bisq/network/p2p/mailbox/MailboxMessageService.java
Outdated
Show resolved
Hide resolved
|
Pr is running on 75% of the seeds since a few days. I guess its safe now to merge into master... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For better resiliance in case all seed nodes would go offline, we persist the mailbox messages and republish them at startup.
We had already a persistence for own mailbox messages and reuse that but add all mailbox messages.
We check for the TTL to not publish expired messages as well we check if we have received a remove message for that mailbox message.
To not overload too much the P2pService class I refactored the mailbox related code out to a new class. The first commits are refactoring commits. The code for the new feature starts at b91a1f7