From 0861a7bae331e14846b3fa1aebaf8fb16433db91 Mon Sep 17 00:00:00 2001
From: Benoit Lavenier <benoit.lavenier@e-is.pro>
Date: Fri, 16 Jun 2023 11:43:33 +0200
Subject: [PATCH] enh(dal): pass the conf to dal.saveBlock(), to let dal decide
 if TXs should be stored or not fix(dal): Split migration step 27, to be able
 to migrate content if alter table failed fix(conf): apply storage defaults
 when creating a ConfDTO fix(sync): clarify code on bindex size fix(sync):
 trim index and sandboxes only on the last block - close #1450

---
 app/lib/blockchain/DuniterBlockchain.ts       | 32 ++++++--------
 app/lib/dal/fileDAL.ts                        | 17 ++++----
 app/lib/dal/sqliteDAL/MetaDAL.ts              | 14 +++++--
 app/lib/dto/ConfDTO.ts                        |  4 ++
 app/lib/indexer.ts                            |  3 ++
 app/modules/crawler/lib/sync.ts               |  7 ++--
 .../crawler/lib/sync/v2/GlobalIndexStream.ts  | 42 ++++++++-----------
 test/dal/triming-dal.ts                       | 20 +++++++--
 8 files changed, 75 insertions(+), 64 deletions(-)

diff --git a/app/lib/blockchain/DuniterBlockchain.ts b/app/lib/blockchain/DuniterBlockchain.ts
index 169f79f58..86f7a7111 100644
--- a/app/lib/blockchain/DuniterBlockchain.ts
+++ b/app/lib/blockchain/DuniterBlockchain.ts
@@ -320,19 +320,12 @@ export class DuniterBlockchain {
     // Compute to be revoked members
     await this.computeToBeRevoked(indexes.mindex, dal);
 
-    // Delete eventually present transactions
-    // NOT NEED => already done in saveTxsInFiles()
-    //await this.deleteTransactions(block, dal);
-
-    await dal.trimSandboxes(block);
+    if (trim) {
+      await dal.trimSandboxes(block);
+    }
 
     // Saves the block (DAL)
-    await dal.saveBlock(dbb);
-
-    // Save Transactions (DAL)
-    if (conf.storage?.transactions !== false) {
-      await dal.saveTxsInFiles(dbb.transactions, dbb.number, dbb.medianTime);
-    }
+    await dal.saveBlock(dbb, conf);
 
     // Save wot file
     if (!dal.fs.isMemoryOnly()) {
@@ -711,6 +704,7 @@ export class DuniterBlockchain {
     const TAIL = await dal.bindexDAL.tail();
     const MAX_BINDEX_SIZE = requiredBindexSizeForTail(TAIL, conf);
     const currentSize = HEAD.number - TAIL.number + 1;
+
     if (currentSize > MAX_BINDEX_SIZE) {
       await dal.trimIndexes(HEAD.number - MAX_BINDEX_SIZE);
     }
@@ -721,13 +715,11 @@ export function requiredBindexSizeForTail(
   TAIL: { issuersCount: number; issuersFrame: number },
   conf: { medianTimeBlocks: number; dtDiffEval: number; forksize: number }
 ) {
-  const bindexSize = [
-    TAIL.issuersCount,
-    TAIL.issuersFrame,
-    conf.medianTimeBlocks,
-    conf.dtDiffEval,
-  ].reduce((max, value) => {
-    return Math.max(max, value);
-  }, 0);
-  return conf.forksize + bindexSize;
+  return conf.forksize +
+      [
+        TAIL.issuersCount,
+        TAIL.issuersFrame,
+        conf.medianTimeBlocks,
+        conf.dtDiffEval,
+      ].reduce((max, value) => Math.max(max, value), 0);
 }
diff --git a/app/lib/dal/fileDAL.ts b/app/lib/dal/fileDAL.ts
index abefa673b..7964a8941 100644
--- a/app/lib/dal/fileDAL.ts
+++ b/app/lib/dal/fileDAL.ts
@@ -213,11 +213,6 @@ export class FileDAL implements ServerDAO {
     }
   }
 
-  generateUpgradeSql() {
-    // Make sure to always renable constraints (a.g. if the last sync failed, it can be still disabled)
-    return "PRAGMA ignore_check_constraints = true;";
-  }
-
   async disableCheckConstraints() {
     logger.info("Disabling database check constraints...");
     await this.metaDAL.exec("PRAGMA ignore_check_constraints = true;");
@@ -1235,11 +1230,13 @@ export class FileDAL implements ServerDAO {
     }
   }
 
-  saveBlock(dbb: DBBlock) {
-    return this.blockDAL.saveBlock(dbb);
+  async saveBlock(dbb: DBBlock, conf: ConfDTO) {
+    await this.blockDAL.saveBlock(dbb);
 
-    // Since v1.8.7, saveTxsInFiles() should be call only if TX storage enabled, by the caller
-    //await this.saveTxsInFiles(dbb.transactions, dbb.number, dbb.medianTime);
+    // Since v1.8.7, TX are not always stored
+    if (conf.storage?.transactions !== false) {
+      await this.saveTxsInFiles(dbb.transactions, dbb.number, dbb.medianTime);
+    }
   }
 
   saveSideBlock(block: DBBlock) {
@@ -1312,6 +1309,7 @@ export class FileDAL implements ServerDAO {
 
   @MonitorExecutionTime()
   async trimIndexes(maxNumber: number) {
+    logger.trace('Trim indexes below block #%s', maxNumber)
     if (!cliprogram.notrim) {
       await this.bindexDAL.trimBlocks(maxNumber);
       await this.iindexDAL.trimRecords(maxNumber);
@@ -1325,6 +1323,7 @@ export class FileDAL implements ServerDAO {
   }
 
   async trimSandboxes(block: { medianTime: number }) {
+    logger.trace('Trim sandboxes below median time %s', block.medianTime)
     await this.certDAL.trimExpiredCerts(block.medianTime);
     await this.msDAL.trimExpiredMemberships(block.medianTime);
     await this.idtyDAL.trimExpiredIdentities(block.medianTime);
diff --git a/app/lib/dal/sqliteDAL/MetaDAL.ts b/app/lib/dal/sqliteDAL/MetaDAL.ts
index 7784a17cf..76047463e 100644
--- a/app/lib/dal/sqliteDAL/MetaDAL.ts
+++ b/app/lib/dal/sqliteDAL/MetaDAL.ts
@@ -220,9 +220,17 @@ export class MetaDAL extends AbstractSQLite<DBMeta> {
       try {
         await txsDAL.exec(
           "ALTER TABLE txs ADD COLUMN issuer VARCHAR(50) NULL;" +
-            "ALTER TABLE txs ADD COLUMN recipient VARCHAR(50) NULL;" +
-            // SHOULD start transaction after ALTER TABLE, to avoid leaving a not closed transaction, if failed - close #1448
-            "BEGIN;" +
+            "ALTER TABLE txs ADD COLUMN recipient VARCHAR(50) NULL;"
+        );
+      } catch (err) {
+        // Silent: if column already exists
+      }
+
+      // Fill columns 'issuer' and 'recipient'
+      // SHOULD start transaction after ALTER TABLE, to avoid leaving a not closed transaction, if failed - close #1448
+      try {
+        await txsDAL.exec(
+          "BEGIN;" +
             "UPDATE txs SET issuer = SUBSTR(issuers, 2, LENGTH(issuers) - 4) WHERE issuer IS NULL AND issuers NOT LIKE '%,%';" +
             "UPDATE txs SET recipient = SUBSTR(recipients, 2, LENGTH(recipients) - 4) WHERE recipient IS NULL AND recipients NOT LIKE '%,%';" +
             "COMMIT;"
diff --git a/app/lib/dto/ConfDTO.ts b/app/lib/dto/ConfDTO.ts
index 6449d4edd..53b33201a 100644
--- a/app/lib/dto/ConfDTO.ts
+++ b/app/lib/dto/ConfDTO.ts
@@ -300,6 +300,10 @@ export class ConfDTO
       forksize: constants.BRANCHES.DEFAULT_WINDOW_SIZE,
       switchOnHeadAdvance: CommonConstants.SWITCH_ON_BRANCH_AHEAD_BY_X_BLOCKS,
       nonWoTPeersLimit: CommonConstants.DEFAULT_NON_WOT_PEERS_LIMIT,
+      storage: {
+        transactions: true,
+        wotwizard: false,
+      },
     };
   }
 
diff --git a/app/lib/indexer.ts b/app/lib/indexer.ts
index 6a01b03df..8759a80d8 100644
--- a/app/lib/indexer.ts
+++ b/app/lib/indexer.ts
@@ -600,6 +600,9 @@ export class Indexer {
 
     const HEAD_1 = await head(1);
 
+    // CHECK Bindex is valid
+    if (HEAD_1 && HEAD_1.number !== HEAD.number - 1) throw new Error('Invalid bindex: cannot found HEAD-1!');
+
     // BR_G04
     await Indexer.prepareIssuersCount(HEAD, range, HEAD_1);
 
diff --git a/app/modules/crawler/lib/sync.ts b/app/modules/crawler/lib/sync.ts
index 45bab02f9..6a5d4dded 100644
--- a/app/modules/crawler/lib/sync.ts
+++ b/app/modules/crawler/lib/sync.ts
@@ -248,9 +248,6 @@ export class Synchroniser extends stream.Duplex {
         await this.syncStrategy.syncPeers(fullSync, to);
       }
 
-      // Enable check constraints
-      if (!cautious) await this.server.dal.enableCheckConstraints();
-
       const syncDuration = Date.now() - syncStartTime;
       this.watcher.end(syncDuration);
       this.push({ sync: true });
@@ -264,5 +261,9 @@ export class Synchroniser extends stream.Duplex {
       this.watcher.end();
       throw err;
     }
+    finally {
+      // Make sure to enable check constraints, even if failed
+      await this.server.dal.enableCheckConstraints();
+    }
   }
 }
diff --git a/app/modules/crawler/lib/sync/v2/GlobalIndexStream.ts b/app/modules/crawler/lib/sync/v2/GlobalIndexStream.ts
index f27fb9149..48a28c8cc 100644
--- a/app/modules/crawler/lib/sync/v2/GlobalIndexStream.ts
+++ b/app/modules/crawler/lib/sync/v2/GlobalIndexStream.ts
@@ -38,8 +38,6 @@ let sync_iindex: any[] = [];
 let sync_mindex: any[] = [];
 let sync_cindex: any[] = [];
 let sync_nextExpiring = 0;
-let sync_bindexSize = 0;
-let sync_txs: any[] = [];
 let txCount = 0;
 let logger = NewLogger();
 
@@ -205,9 +203,9 @@ export class GlobalIndexStream extends Duplex {
         );
       }
 
-      const bindexSize = requiredBindexSizeForTail(block, this.conf);
+      const requiredBindexSize = requiredBindexSizeForTail(block, this.conf);
       if (
-        (block.number <= this.to - bindexSize - 1 || cliprogram.noSources) &&
+        (block.number <= this.to - requiredBindexSize - 1 || cliprogram.noSources) &&
         !this.cautious
       ) {
         // If we require nosources option, this blockchain can't be valid so we don't make checks
@@ -392,27 +390,21 @@ export class GlobalIndexStream extends Duplex {
         }
 
         // Trim the bindex
-        sync_bindexSize =
-          this.conf.forksize +
-          [
-            block.issuersCount,
-            block.issuersFrame,
-            this.conf.medianTimeBlocks,
-            this.conf.dtDiffEval,
-            dataArray.length,
-          ].reduce((max, value) => {
-            return Math.max(max, value);
-          }, 0);
-
-        if (sync_bindexSize && sync_bindex.length >= 2 * sync_bindexSize) {
+        let maxBindexSize = Math.max(requiredBindexSize, dataArray.length) * 2;
+
+        if (sync_bindex.length >= maxBindexSize) {
           // We trim it, not necessary to store it all (we already store the full blocks)
-          sync_bindex.splice(0, sync_bindexSize);
-          // TODO GINDEX
-          await this.doTrimming();
+          sync_bindex.splice(0, sync_bindex.length - requiredBindexSize);
+          await this.trimIndexes();
         }
       } else if (block.number <= this.to) {
+        // Trim bindex to the minimal required size
+        if (sync_bindex.length > requiredBindexSize) {
+          sync_bindex.splice(0, sync_bindex.length - requiredBindexSize);
+        }
+
         const dto = BlockDTO.fromJSONObject(block);
-        await this.finalizeSync(block, dto);
+        await this.finalizeSync(block, dto, dto.number === this.to);
       }
 
       gindex.push(gData);
@@ -530,13 +522,13 @@ export class GlobalIndexStream extends Duplex {
   }
 
   @MonitorExecutionTime()
-  private async doTrimming() {
+  private async trimIndexes() {
     // Process triming & archiving continuously to avoid super long ending of sync
     await this.dal.trimIndexes(sync_bindex[0].number);
   }
 
   @MonitorExecutionTime()
-  private async finalizeSync(block: BlockDTO, dto: BlockDTO) {
+  private async finalizeSync(block: BlockDTO, dto: BlockDTO, trim: boolean) {
     // Save the INDEX
     await this.dal.bindexDAL.insertBatch(sync_bindex);
     await this.dal.flushIndexes({
@@ -607,7 +599,8 @@ export class GlobalIndexStream extends Duplex {
       HEAD,
       this.conf,
       this.dal,
-      NewLogger()
+      NewLogger(),
+      trim
     );
 
     // Clean temporary variables
@@ -615,7 +608,6 @@ export class GlobalIndexStream extends Duplex {
     sync_iindex = [];
     sync_mindex = [];
     sync_cindex = [];
-    sync_bindexSize = 0;
     sync_expires = [];
     sync_nextExpiring = 0;
   }
diff --git a/test/dal/triming-dal.ts b/test/dal/triming-dal.ts
index 1fc86192e..0a25ea161 100644
--- a/test/dal/triming-dal.ts
+++ b/test/dal/triming-dal.ts
@@ -16,6 +16,7 @@ import {Directory} from "../../app/lib/system/directory"
 import {Indexer} from "../../app/lib/indexer"
 import {simpleNodeWith2Users} from "../integration/tools/toolbox"
 import {LevelDBSindex} from "../../app/lib/dal/indexDAL/leveldb/LevelDBSindex";
+import {requiredBindexSizeForTail} from "../../app/lib/blockchain/DuniterBlockchain";
 
 const should = require('should');
 
@@ -173,19 +174,30 @@ describe("Triming", function(){
 
   it('should be able to trim the bindex', async () => {
     // Triming
-    const server = (await simpleNodeWith2Users({
+    const conf = {
       forksize: 9,
       sigQty: 1,
       dtDiffEval: 2,
       medianTimeBlocks: 3
-    })).s1;
+    };
+    const server = (await simpleNodeWith2Users(conf)).s1;
+
     // const s1 = server.s1;
-    for (let i = 0; i < 13; i++) {
+    const b0 = await server.commit()
+
+    const requiredBindexSize = requiredBindexSizeForTail(b0, conf);
+    should(requiredBindexSize).equal(12);
+
+    for (let i = 1; i <= requiredBindexSize; i++) {
       await server.commit();
     }
     (await server.dal.bindexDAL.head(1)).should.have.property('number').equal(12);
     (await server.dal.bindexDAL.head(13)).should.have.property('number').equal(0);
-    await server.commit();
+
+    // Fill bindex until requiredBindexSize * 2
+    for (let i = 0; i < requiredBindexSize; i++) {
+      await server.commit();
+    }
     should.not.exists(await server.dal.bindexDAL.head(14)); // Trimed
 
     await server.closeCluster()
-- 
GitLab