[bisq-network/bisq] Reduce initial request size (#4233)

[sqrrm]

@sqrrm commented on this pull request.

Thinking a bit about it I guess there is conceptually the same model we have now with one store, just split in many.

I am having a hard time getting a good overview of what's going on, but I think there is some muddling of concerns. A new client would not have to know about the old format as I see it. Only those nodes delivering data (seednodes typically) need to separate the kind of request they're getting and deliver data accordingly.

> -     */
-    private String extractSpecialKey(String name, String postFix) {
-        return name.replace(postFix, "") // remove the postfix
-                .replace("_", "") // remove the spacings
-                .replace(getFileName(), ""); // remove the file name
+        // load our live data store
+        store = readStore(getFileName());
+
+        // create file list of files we should have by now
+        List<String> versions = new ArrayList<>();
+        versions.add(Version.VERSION);
+        versions.addAll(Version.history);
+
+        // go through the list one by one
+        versions.forEach(version -> {
+            String filename = getFileName() + "_" + version + postFix; // postFix has a preceding "_"

There aren't any postfixes in the db dir, should the postFix be added here?

> +        store = readStore(getFileName());
+
+        // create file list of files we should have by now
+        List<String> versions = new ArrayList<>();
+        versions.add(Version.VERSION);
+        versions.addAll(Version.history);
+
+        // go through the list one by one
+        versions.forEach(version -> {
+            String filename = getFileName() + "_" + version + postFix; // postFix has a preceding "_"
+            if (new File(absolutePathOfStorageDir, filename).exists()) {
+                // if it is there already, load
+                history.put(version, readStore(getFileName() + "_" + version));
+            } else {
+                // either copy and split
+                history.put(version, copyAndSplit(version, postFix));

This will be done for all historical versions but copyAndSplit doesn't know about versions so it will make an identical copy of the resource data and put a version number on it.

I think there is something not right with this. We should be shipping this version with a split store (which will just be one old version resource). As time goes we will add more versioned resource files. It might be the old storage format doesn't have to ported in code at all. If we just rename the resource files we have to name_1.3.4_BTC_MAINNET this version is already good to go.


> -            return result.get();
-        else
-            return "";
+    /**
+     * See if the request contains a "special key".
+     *
+     * @param knownPayloadHashes
+     * @throws NoSuchElementException if there is no "special key" in the list
+     * @return the "special key"
+     */
+    private String containsSpecialKey(Set<P2PDataStorage.ByteArray> knownPayloadHashes) {
+        return knownPayloadHashes.stream()
+                .map(byteArray -> new String(byteArray.bytes).trim())
+                .filter(s -> s.matches("^[0-9]\\.[0-9]\\.[0-9]$"))
+                .findFirst()
+                .orElseThrow();

Please don't use exceptions as part of the expected execution path. This is not how Bisq uses exceptions so it causes confusion.

-- 
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/4233#pullrequestreview-416146390
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200521/c47c870e/attachment.html>