[bisq-network/bisq] (6/6) Clean up technical debt in P2PDataStorage and ProtectedStorageEntry objects (#3747)
Christoph Atteneder
notifications at github.com
Mon Dec 9 12:10:59 UTC 2019
ripcurlx commented on this pull request.
> - if (protectedStorageEntry.isExpired(this.clock)) {
- log.info("We found an expired data entry which we have already back dated. " +
- "We remove the protectedStoragePayload:\n\t" + Utilities.toTruncatedString(protectedStorageEntry.getProtectedStoragePayload(), 100));
- removeFromMapAndDataStore(protectedStorageEntry, hashOfPayload);
- }
- } else {
- log.debug("Remove data ignored as we don't have an entry for that data.");
- }
- }
- }
- });
- }
+ if (closeConnectionReason.isIntended)
+ return;
+
+ if (!connection.getPeersNodeAddressOptional().isPresent())
Is there a reason why you prefer using directly `getPeersNodeAddressOptional().isPresent()` in comparison using `hasPeerNodeAddress()` ? It is used mixed in the original code as well, so we might use one way for the future.
> + NodeAddress peersNodeAddress = connection.getPeersNodeAddressOptional().get();
+
+ // Retrieve all the eligible payloads based on the node that disconnected
+ ArrayList<Map.Entry<ByteArray, ProtectedStorageEntry>> toBackDate =
+ map.entrySet().stream()
+ .filter(entry -> entry.getValue().getProtectedStoragePayload() instanceof ExpirablePayload)
+ .filter(entry -> entry.getValue().getProtectedStoragePayload() instanceof RequiresOwnerIsOnlinePayload)
+ .filter(entry -> ((RequiresOwnerIsOnlinePayload) entry.getValue().getProtectedStoragePayload()).getOwnerNodeAddress().equals(peersNodeAddress))
+ .collect(Collectors.toCollection(ArrayList::new));
+
+ // Backdate each payload
+ toBackDate.forEach(mapEntry -> {
+ // We only set the data back by half of the TTL and remove the data only if is has
+ // expired after that back dating.
+ // We might get connection drops which are not caused by the node going offline, so
+ // we give more tolerance with that approach, giving the node the change to
NIT: Small typo in the original code: `change` should be `chance`
> - Utilities.toTruncatedString(toRemoveItem.getValue()));
- });
- removeFromMapAndDataStore(toRemoveList);
-
- if (sequenceNumberMap.size() > this.maxSequenceNumberMapSizeBeforePurge)
- sequenceNumberMap.setMap(getPurgedSequenceNumberMap(sequenceNumberMap.getMap()));
+ // Backdate all the eligible payloads based on the node that disconnected
+ map.values().stream()
+ .filter(protectedStorageEntry -> protectedStorageEntry.getProtectedStoragePayload() instanceof ExpirablePayload)
+ .filter(protectedStorageEntry -> protectedStorageEntry.getProtectedStoragePayload() instanceof RequiresOwnerIsOnlinePayload)
+ .filter(protectedStorageEntry -> ((RequiresOwnerIsOnlinePayload) protectedStorageEntry.getProtectedStoragePayload()).getOwnerNodeAddress().equals(peersNodeAddress))
+ .forEach(protectedStorageEntry -> {
+ // We only set the data back by half of the TTL and remove the data only if is has
+ // expired after that back dating.
+ // We might get connection drops which are not caused by the node going offline, so
+ // we give more tolerance with that approach, giving the node the change to
NIT: The `change` != `chance` typo in the original code is copied over to this place as well.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3747#pullrequestreview-328870009
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191209/a2d25839/attachment.html>
More information about the bisq-github
mailing list