blink icon indicating copy to clipboard operation
blink copied to clipboard

Medici version 5

Open Uzlopak opened this issue 3 years ago • 10 comments
trafficstars

Hi,

I worked the last couple of days on refactoring medici. It is now written in typescript and has a code coverage of 100%.

https://github.com/flash-oss/medici/issues/34

It can use ACID-Transactions and will have a better precision handling than the current implementation.

I hope that @koresar will release the new version next week.

It would be cool if you have a look at it. https://github.com/flash-oss/medici/tree/typescript-migration

The current changes cover following fixes and added features:

  • Add support for MongoDB sessions (aka ACID transactions). See IOptions type.
  • Added a mongoTransaction-method, which is a convenience shortcut for mongoose.connection.transaction.
  • Added async helper method initModels, which initializes the underlying transactionModel and journalModel. Use this after you connected to the MongoDB-Server if you want to use transactions. Or else you could get Unable to read from a snapshot due to pending collection catalog changes; please retry the operation.-Error when acquiring a session because the actual database-collection is still being created by the underlying mongoose-instance.
  • Node.js 12 is the lowest supported version. Although, 10 should still work fine, when using mongoose v5.
  • Mongoose v6 is the only supported version now. Avoid using both v5 and v6 in the same project.
  • MongoDB 4 and above is supported. You can still use MongoDB 3, but ACID-sessions could have issues.
  • You can't import book anymore. Only Book is supported. require("medici").Book.
  • The project was rewritten with TypeScript. Types are provided within the package now.
  • .ledger() returns lean Transaction-Objects for better performance. To retrieve hydrated Transaction-Objects, set lean to false in the third parameter of .ledger(). It is recommended to not hydrate the transactions, as it implies that the transactions could be manipulated and the data integrity of Medici could be risked.
  • You can now specify the precision. Book now accepts an optional second parameter, where you can set the precision used internally by Medici. Default value is 7 digits precision. Javascript has issues with floating points precision and can only handle 16 digits precision, like 0.1 + 0.2 results in 0.30000000000000004 and not 0.3. The default precision of 7 digits after decimal, results in the correct result of 0.1 + 0.2 = 0.3. The default value is taken from medici version 4.0.2. Be careful, if you use currency, which has more decimal points, e.g. Bitcoin has a precision of 8 digits after the comma. So for Bitcoin you should set the precision to 8. You can enforce an "only-Integer"-mode, by setting the precision to 0. But keep in mind that Javascript has a max safe integer limit of 9007199254740991.
  • Added setJournalSchema and setTransactionSchema to use custom Schemas. It will ensure, that all relevant middlewares and methods are also added when using custom Schemas.
  • Added prototype-pollution protection when creating entries. Reserved words like __proto__ can not be used as properties of a Transaction or a Journal or their meta-Field as they will get silently filtered.

Uzlopak avatar Dec 18 '21 00:12 Uzlopak

You are welcome for code review https://github.com/flash-oss/medici/pull/35

Uzlopak avatar Dec 18 '21 00:12 Uzlopak

Updated Changelog:

  • v5.0.0

    • The project was rewritten with TypeScript. Types are provided within the package now.
    • Added support for MongoDB sessions (aka ACID transactions). See IOptions type.
    • Added a mongoTransaction-method, which is a convenience shortcut for mongoose.connection.transaction.
    • Added async helper method initModels, which initializes the underlying transactionModel and journalModel. Use this after you connected to the MongoDB-Server if you want to use transactions. Or else you could get Unable to read from a snapshot due to pending collection catalog changes; please retry the operation.-Error when acquiring a session because the actual database-collection is still being created by the underlying mongoose-instance.
    • Node.js 12 is the lowest supported version. Although, 10 should still work fine, when using mongoose v5.
    • Mongoose v6 is the only supported version now. Avoid using both v5 and v6 in the same project.
    • MongoDB 4 and above is supported. You can still use MongoDB 3, but ACID-sessions could have issues.
    • You can't import book anymore. Only Book is supported. require("medici").Book.
    • Added a new index on the transactionModel to improve paginated ledger queries.
    • .ledger() returns lean Transaction-Objects for better performance. To retrieve hydrated Transaction-Objects, set lean to false in the third parameter of .ledger(). It is recommended to not hydrate the transactions, as it implies that the transactions could be manipulated and the data integrity of Medici could be risked.
    • You can now specify the precision. Book now accepts an optional second parameter, where you can set the precision used internally by Medici. Default value is 7 digits precision. Javascript has issues with floating points precision and can only handle 16 digits precision, like 0.1 + 0.2 results in 0.30000000000000004 and not 0.3. The default precision of 7 digits after decimal, results in the correct result of 0.1 + 0.2 = 0.3. The default value is taken from medici version 4.0.2. Be careful, if you use currency, which has more decimal points, e.g. Bitcoin has a precision of 8 digits after the comma. So for Bitcoin you should set the precision to 8. You can enforce an "only-Integer"-mode, by setting the precision to 0. But keep in mind that Javascript has a max safe integer limit of 9007199254740991.
    • Added maxAccountPath. You can set the maximum amount of account paths via the second parameter of Book. This can improve the performance of .balance() and .ledger() calls as it will then use the accounts attribute of the transactions as a filter.
    • Added validation for name of Book, maxAccountPath and precision. A name has to be not an empty string or a string containing only whitespaces. precision has to be an integer bigger or equal 0. maxAccountPath has to be an integer bigger or equal 0.
    • Added setJournalSchema and setTransactionSchema to use custom Schemas. It will ensure, that all relevant middlewares and methods are also added when using custom Schemas. Use syncIndexes-method from medici after setTransactionSchema to enforce the defined indexes on the models.
    • Added prototype-pollution protection when creating entries. Reserved words like __proto__ can not be used as properties of a Transaction or a Journal or their meta-Field as they will get silently filtered.
    • When calling book.void() the provided journal_id has to belong to the book. If the journal does not exist within the book, medici will throw a JournalNotFoundError. In medici < 5 you could theoretically void a journal of another book.
    • Added a trackAccountChanges-model to make it possible to call .balance() while using a mongo-session.

Uzlopak avatar Dec 20 '21 13:12 Uzlopak

No worries mate. The changes you did are very well needed. We'll merge all the features you need. It's just Xmas, very busy time of the year for us. We'll get there.

koresar avatar Dec 21 '21 06:12 koresar

@koresar Dont worry. I am not pushing to merge it. Here in the galoy repo I try to get the galoy developers to have a look into the potential changes. Maybe they want also some changes specific for their needs.

Yesterday I invested the half day to figure out how we can ensure that accounts can not fall below 0 while being in a transaction (avoiding double spending), because @nicolasburtey mentioned they would like to have that feature. So the feature is now implemented :).

A typescript example:

import { Book, mongoTransaction } from "medici";

const mainLedger = new Book("mainLedger");

async function withdraw(walletId: string, amount: number) {
  return mongoTransaction(session => {

      await mainLedger
        .entry("Withdraw by User")
        .credit("Assets", amount)
        .debit(`Accounts:${walletId}`, amount)
        .commit({ session });

      // .balance() can be an resource-expensive operation. So we do it after we
      // created the journal.
      const balanceAfter = await mainLedger.balance(
        {
          account: `Accounts:${walletId}`,
        },
        { session }
      );

      // Avoid spending more than the wallet has
      if (balanceAfter.balance < 0) {
        throw new Error("Not enough balance in wallet.");
      }

      // MongoDB Performance Tuning (2021), p. 217
      // Reduce the Chance of Transient Transaction Errors by moving the
      // contentious statement to the end of the transaction.

      // We writelock only the account of the User/Wallet. If we writelock a very 
      // often used account, like the fictitious Assets account in this example, 
      // we would slow down the database extremely as the writelocks would make
      // it impossible to concurrently write in the database.
      // We only check the balance of the User/Wallet, so only this Account has to
      // be writelocked.
      await mainLedger.writelockAccounts([`Accounts:${walletId}`], { session });
  });
}

Uzlopak avatar Dec 21 '21 14:12 Uzlopak

For best performance I recommend that galoy sets the maxAccountPath to 2 if it is only using 2 paths. And should probably use a different order of the account_path indexes to have them sorted by cardinality. I wrote about cardinality in the readme.md

Uzlopak avatar Dec 21 '21 14:12 Uzlopak

Imo it would be much better to implement this using optimistic concurrency on the journal collection instead of locking via an additional Account Collection.

Perhaps we can have a chat about this @Uzlopak on zoom

for context: https://stackoverflow.com/questions/129329/optimistic-vs-pessimistic-locking

bodymindarts avatar Dec 22 '21 10:12 bodymindarts

AFAIK, distributed ACID transactions work in the optimistic way. This last writelockAccounts() is doing the version number check and will throw if number has changed.

Distributed ACID transactions do not lock anything per se. Although, if load is high you get a lot of TransientTransactionError.

koresar avatar Dec 22 '21 11:12 koresar

Let me talk about this line though:

      const balanceAfter = await mainLedger.balance(
        {
          account: `Accounts:${walletId}`,
        },
        { session }
      );

We have a "wallet" in our system with a million ledger transactions. This balance() check takes from 0.5 to 3 seconds.

I would not recommend the above approach to protect from double spending. I would rather keep the balance in a separate synchronised collection. Just like the "MongoDB Performance Tuning" depicts.

koresar avatar Dec 22 '21 11:12 koresar

The issue is, that the journals are all independent. They don't store the affected accounts nor any data which can be something like the preceding journal with the affected accounts. Also would make no sense, as a journal could contain multiple transactions and affected accounts.

So I don't see any opportunity to persist something on the journal.

Also persisting something like an account collection would also affect the overall performance as a heavy used account would result in constantly having locked records.

There is imho no real optimistic locking strategy possible. E.g. even with an account model you would implicitly want to even update heavy used accounts as you want the accounts having always the correct value. So now you degraded the write operation for everyone.

I don't say that balance is most performant. But with writelockAccounts you can select what you actually want to writelock. If you implement a better balance method, it would be independent from the locking. But on the other hand, that would by itself could be also resulting in a writelock.

Uzlopak avatar Dec 22 '21 15:12 Uzlopak

Medici v5 was released. Should be non-breaking upgrade for galoy. See docs: https://github.com/flash-oss/medici#v500

High level overview.

  • The project was rewritten with TypeScript. Types are provided within the package now.
  • Added support for MongoDB sessions (aka ACID transactions). See IOptions type.
  • Did number of consistency, stability, server disk space, and speed improvements. Balance querying on massive databases with millions of documents are going to be much-much faster now. Like 10 to 1000 times faster.
  • Mongoose v5 and v6 are both supported now.

koresar avatar Jan 13 '22 01:01 koresar

this has been merged

nicolasburtey avatar Aug 31 '22 21:08 nicolasburtey