[bisq-network/bisq] Bundle messages at broadcast (#4436)

[sqrrm]

@sqrrm requested changes on this pull request.

I think this looks pretty rather solid. I have run my live node getting good stats with no issues seen as well
![image](https://user-images.githubusercontent.com/23560607/91664594-9faa2500-eaf0-11ea-8e72-1a2a40d5e914.png)

Just some small comments that would be good to tend to.

>                            @Nullable BroadcastHandler.Listener listener) {
-        BroadcastHandler broadcastHandler = new BroadcastHandler(networkNode, peerManager);
-        broadcastHandler.broadcast(message, sender, this, listener);
-        broadcastHandlers.add(broadcastHandler);
+        broadcastRequests.add(new BroadcastRequest(message, sender, listener));
+        // Keep that log on INFO for better debugging if the feature works as expected. Later it can
+        // be remove or set to DEBUG
+        log.info("Broadcast requested for {}. We queue it up for next bundled broadcast.",

This is rather verbose, not sure it's a good idea to keep level at info.

After running a mainnet client for a while this log is 40% of the total log lines.

> +            if (maxPossibleSuccessCases == numOfCompletedBroadcastsTarget - 1) {
+                broadcastRequests.stream()
+                        .filter(broadcastRequest -> broadcastRequest.getListener() != null)
+                        .map(Broadcaster.BroadcastRequest::getListener)
+                        .forEach(listener -> listener.onNotSufficientlyBroadcast(numOfCompletedBroadcasts, numOfFailedBroadcasts));
+            } else if (timeoutTriggered && numOfCompletedBroadcasts < numOfCompletedBroadcastsTarget) {
+                // We did not reach resilience level and timeout prevents to reach it later
+                broadcastRequests.stream()
+                        .filter(broadcastRequest -> broadcastRequest.getListener() != null)
+                        .map(Broadcaster.BroadcastRequest::getListener)
+                        .forEach(listener -> listener.onNotSufficientlyBroadcast(numOfCompletedBroadcasts, numOfFailedBroadcasts));
+            }

These two cases could be combined to one block

-- 
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/4436#pullrequestreview-478199394
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200830/50379699/attachment.html>