Reverting a block will never trim LevelDBSindex.indexForTrimming

In LevelDBSindex.removeBlock() the function trimWrittenOn() is supposed to clean entries of the index indexForTrimming but instead indexForConditions is used :

  private async trimWrittenOn(writtenOn: number, id: string) {
    const k = LevelDBSindex.trimWrittenOnKey(writtenOn);
    const existing = await this.getWrittenOnSourceIds(writtenOn);
    const trimmed = arrayPruneAllCopy(existing, id);
    if (trimmed.length) {
      await this.indexForConditionsput(k, trimmed); // <= SHOULD BE indexForTrimming
    } else {
      await this.indexForConditions.del(k); // <= SHOULD BE indexForTrimming
    }
  }

assigned to @c-geek

changed title from LevelDBSindex.indexForTrimming is enever trimmed to Reverting a block will never trim LevelDBSindex.indexForTrimming

mentioned in commit 3ba60c15

mentioned in merge request !1434 (closed)

mentioned in commit 8ef61d86

Non c'est bien indexForConditions l'index des UTXO, c'est celui qu'on souhaite élaguer.

Tandis que indexForTrimming sert à consigner ce qui pourra être élagué (c'est l'index qui contient les sources écrites au blockstamp fourni.

edit : d'ailleurs indexForTrimming se fait bien élaguer lui aussi, à coup de indexForTrimming.del().

Je ne comprends pas, si c'est indexForConditions, la clef doit être un condition, et non un id basé sur writtenOn, non ? Vraiment, je penses que cette fonction n'est pas correcte. Elle est appellé uniquement à la fin de élagage < blockNumber, tout à la fin. C'est donc bien l'endroit pour nettoyer indexForTrimming. les autre del() que je vois sont sur les revert de blocs.

Tu me mets le doute. Ecris un test et nous serons fixés. :-)

mentioned in commit ef5707e6

@c-geek voila j'ai ajouté un test (sur la branche release/1.8) :

• Ce test should be able to trim the sindex
  • fait un élagave avant le bloc 140, et test qu'il reste bien les 2 sources non consommées SOURCE_2 et SOURCE_3 (SOURCE_1 a été consommé et supprimer par l'élagage)
  • consomme ensuite les dernières sources.
  • élagage à nouveau
  • vérifie que tous les sous-index (dont indexForTrimming) sont vides

/!\ Pour reproduire :

• remettre d'abord indexForConditions dans trimWrittenOn(), car j'avais mergé ma correction (que je crois bonne sur ce point)

/!\ Malgré la correction, le test unitaire ne passe pas, mais pour d'autres raisons :

• en fait le premier élagage ne fonctionne pas bien non plus.
• en débugguant, je m'apercois que indexForTrimming a toujours la SOURCE_1 pour le bloc 126. Seule le bloc 139 (qui consomme la SOURCE_1) a été nettoyé.
• Ceci voudrait dire qu'indexForTrimming conserve toujours toutes les sources de type CREATE.

Bref, ma correction améliore une peu l'élagage (avant tout était conservé !) mais ne résoud pas tout.

Pour le problème restant, je viens d'ajouter une vérification dans le test (mise en commentaire avec un FIXME, afin que tu puisses regarder les problèmes dans l'ordre)

mentioned in commit 67b05f1a

ok je penses que j'ai corrigé le problème restant, via le commit 67b05f1a Le cas de test est maintenant complet

Effectivement, merci pour le test j'ai pu y voir plus clair. J'ai apporté quelques modifications que j'ai rajouté en local pour me convaincre.

TODO : apply the fix in the dev branch

Par contre j'ai encore un doute sur la ligne if (createRecord && updateRecord && updateRecord.writtenOn < belowNumber) {, je crains qu'on ne supprime pas certaines sources si createRecord n'existe pas. Je ne sais pas si c'est possible.

edit : non c'est bon, j'ai commenté pour me convaincre là-aussi que c'était impossible de ne pas avoir les 2 enregistrements.

J'ai pushé des commentaires supplémentaires. Je n'ai pas encore pu réfléchir aux conséquences de tout cela.

Donc au final, le bug principal était que indexForTrimming n'était jamais élagué, et il y avait en plus un sur-bug quand tu as tenté de le faire (manquait l'élagage du CREATE).

Après recherches indexForTrimming ne servait qu'au revert de bloc. Donc sans conséquence sur le protocole ni les clients. Par contre, impact sur les performances avec un index qui ne faisait que grossir.

unassigned @c-geek