Skip to content

Commit

Permalink
feat: remove the recently added listAccount() caching as useless afte…
Browse files Browse the repository at this point in the history 
…r the default indexes revamp
  • Loading branch information
@koresar
koresar committed Dec 8, 2023
1 parent 73c6c9a commit e91723c
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 209 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ This release fixes the unfortunate mistake.
- `timestamp` is a read-only field created by `medici` thus `medici` can rely on it.
- but `datetime` is an **arbitrary** value you can pass when creating ledger entries. Almost nobody is doing that though.
- If you need indexes for the `datetime` field then please create yourself.
- Removed the `book.listAccounts()` caching which added in the previous release (v6.3) because the default indexes cover this use case now. Moreover, the index works faster than the cache.

### 6.3

Expand Down
82 changes: 0 additions & 82 deletions spec/book.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,30 +615,13 @@ describe("book", function () {
const book1 = new Book("MyBook-listAccounts sorting 1");
await book1.entry("MyBook-listAccounts sorting 1").debit("Income:Rent Taxable", 1).credit("Assets", 1).commit();
await book1.entry("MyBook-listAccounts sorting 1").credit("Liabilities", 1).debit("Client Custody", 1).commit();
const accounts1 = await book1.listAccounts();

const book2 = new Book("MyBook-listAccounts sorting 2");
await book2.entry("MyBook-listAccounts sorting 2").debit("Client Custody", 2).credit("Liabilities", 2).commit();
await book2.entry("MyBook-listAccounts sorting 2").credit("Assets", 3).debit("Income:Rent Taxable", 3).commit();
const accounts2 = await book2.listAccounts();

expect(accounts1).to.deep.equal(accounts2);
expect(accounts1).to.deep.equal(["Assets", "Client Custody", "Income", "Income:Rent Taxable", "Liabilities"]);
});

it("should sort accounts alphabetically including cached values", async () => {
const book1 = new Book("MyBook-listAccounts sorting 1");
await book1.entry("MyBook-listAccounts sorting 1").debit("Income:Rent Taxable", 1).credit("Assets", 1).commit();
await book1.entry("MyBook-listAccounts sorting 1").credit("Liabilities", 1).debit("Client Custody", 1).commit();
await book1.listAccounts(); // creates cached doc
await book1.entry("MyBook-listAccounts sorting 1").credit("Z", 1).debit("ZZ", 1).commit();
await book1.entry("MyBook-listAccounts sorting 1").credit("A", 1).debit("AA", 1).commit();
const accounts1 = await book1.listAccounts();

const book2 = new Book("MyBook-listAccounts sorting 2");
await book2.entry("MyBook-listAccounts sorting 2").debit("Client Custody", 2).credit("Liabilities", 2).commit();
await book2.entry("MyBook-listAccounts sorting 2").credit("Assets", 3).debit("Income:Rent Taxable", 3).commit();
await book2.listAccounts(); // created cached doc
await book2.entry("MyBook-listAccounts sorting 2").credit("Z", 3).debit("ZZ", 3).commit();
await book2.entry("MyBook-listAccounts sorting 2").credit("A", 3).debit("AA", 3).commit();
const accounts2 = await book2.listAccounts();
Expand Down Expand Up @@ -670,71 +653,6 @@ describe("book", function () {
.commit();
}

it("should reuse the snapshot listAccounts", async () => {
const book = new Book("MyBook-listAccounts-snapshot");
await addBalance(book);

await book.listAccounts();

let snapshots = await balanceModel.find({ book: book.name });
expect(snapshots).to.have.length(1);
expect(snapshots[0].meta.accounts).to.deep.equal(["Assets", "Assets:Receivable", "Income", "Income:Rent"]);

snapshots[0].meta.accounts = ["new-list"];
await snapshots[0].save();

await book.listAccounts();
snapshots = await balanceModel.find({ book: book.name });

expect(snapshots).to.have.length(1);
expect(snapshots[0].meta.accounts).to.deep.equal(["Assets", "Assets:Receivable", "Income", "Income:Rent"]);
});

it("should create only one snapshot document", async () => {
const book = new Book("MyBook-listAccounts-snapshot-count");
await addBalance(book);

await book.listAccounts();
await book.listAccounts();
await book.listAccounts();

const snapshots = await balanceModel.find({ book: book.name });
expect(snapshots).to.have.length(1);
expect(snapshots[0].meta.accounts).to.deep.equal(["Assets", "Assets:Receivable", "Income", "Income:Rent"]);
});

it("should create periodic balance snapshot document", async () => {
const howOften = 50; // milliseconds
const book = new Book("MyBook-listAccounts-snapshot-periodic", { balanceSnapshotSec: howOften / 1000 });

await addBalance(book);

await book.listAccounts();
// Should be one snapshot.
let snapshots = await balanceModel.find({ book: book.name });
expect(snapshots.length).to.equal(1);
expect(snapshots[0].meta.accounts).to.deep.equal(["Assets", "Assets:Receivable", "Income", "Income:Rent"]);

await delay(howOften + 1); // wait long enough to create a second periodic snapshot

await addBalance(book, "2");
await book.listAccounts();
await delay(10); // wait until the full listAccounts snapshot is recalculated in the background

// Should be two snapshots now.
snapshots = await balanceModel.find({ book: book.name });
expect(snapshots.length).to.equal(2);
expect(snapshots[0].meta.accounts).to.deep.equal(["Assets", "Assets:Receivable", "Income", "Income:Rent"]);
expect(snapshots[1].meta.accounts).to.deep.equal([
"Assets",
"Assets:Receivable",
"Assets:Receivable2",
"Income",
"Income:Rent",
"Income:Rent2",
]);
});

it("should not do listAccounts snapshots if turned off", async () => {
const book = new Book("MyBook-balance-listAccounts-off", { balanceSnapshotSec: 0 });
await addBalance(book);
Expand Down
112 changes: 15 additions & 97 deletions src/Book.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,7 @@ import { IJournal, journalModel } from "./models/journal";
import { ITransaction, transactionModel } from "./models/transaction";
import type { IOptions } from "./IOptions";
import { lockModel } from "./models/lock";
import {
getBestBalanceSnapshot,
getBestListAccountsSnapshot,
IBalance,
snapshotBalance,
snapshotListAccounts,
} from "./models/balance";
import { IFilter } from "./helper/parse/IFilter";
import { getBestBalanceSnapshot, IBalance, snapshotBalance } from "./models/balance";

const GROUP = {
$group: {
Expand All @@ -33,20 +26,6 @@ const GROUP = {
},
};

function fromDistinctToAccounts(distinctResult: string[]) {
const accountsSet: Set<string> = new Set();
for (const fullAccountName of distinctResult) {
const paths = fullAccountName.split(":");
let path = paths[0];
accountsSet.add(path);
for (let i = 1; i < paths.length; ++i) {
path += ":" + paths[i];
accountsSet.add(path);
}
}
return Array.from(accountsSet).sort();
}

export class Book<U extends ITransaction = ITransaction, J extends IJournal = IJournal> {
name: string;
precision: number;
Expand Down Expand Up @@ -343,83 +322,22 @@ export class Book<U extends ITransaction = ITransaction, J extends IJournal = IJ
}

async listAccounts(options = {} as IOptions): Promise<string[]> {
// If there is a session, we must NOT set any readPreference (as per mongo v5 and v6).
// https://www.mongodb.com/docs/v6.0/core/transactions/#read-concern-write-concern-read-preference
// Otherwise, we are free to use any readPreference.
if (options && !options.session && !options.readPreference) {
// Let's try reading from the secondary node, if available.
options.readPreference = "secondaryPreferred";
}

const query = { book: this.name } as IFilter;

let listAccountsSnapshot: IBalance | null = null;
if (this.balanceSnapshotSec) {
listAccountsSnapshot = await getBestListAccountsSnapshot({ book: this.name });
if (listAccountsSnapshot) {
// Use cached balance
query.timestamp = { $gte: listAccountsSnapshot.createdAt };
}
}

const createdAt = new Date(); // take date earlier
const results = (await transactionModel.collection.distinct("accounts", query, {
session: options.session,
})) as string[];

let uniqueAccounts = fromDistinctToAccounts(results);

if (listAccountsSnapshot) {
uniqueAccounts = Array.from(new Set([...uniqueAccounts, ...listAccountsSnapshot.meta.accounts])).sort();
}

if (uniqueAccounts.length === 0) return uniqueAccounts;

if (this.balanceSnapshotSec) {
// It's the first (ever?) snapshot for this listAccounts. We just need to save whatever we've just aggregated
// so that the very next listAccounts query would use cached snapshot.
if (!listAccountsSnapshot) {
await snapshotListAccounts({
book: this.name,
accounts: uniqueAccounts,
createdAt,
expireInSec: this.expireBalanceSnapshotSec,
});
} else {
// There is a snapshot already. But let's check if it's too old.
const isSnapshotObsolete =
Date.now() > listAccountsSnapshot.createdAt.getTime() + this.balanceSnapshotSec * 1000;
// If it's too old we would need to cache another snapshot.
if (isSnapshotObsolete) {
delete query.timestamp;

// Important! We are going to recalculate the entire listAccounts from the day one.
// Since this operation can take seconds (if you have millions of documents)
// we better run this query IN THE BACKGROUND.
// If this exact balance query would be executed multiple times at the same second we might end up with
// multiple snapshots in the database. Which is fine. The chance of this happening is low.
// Our main goal here is not to delay this .listAccounts() method call. The tradeoff is that
// database will use 100% CPU for few (milli)seconds, which is fine. It's all fine (C)
transactionModel.collection
.distinct("accounts", query, {
session: options.session,
})
.then((results) =>
snapshotListAccounts({
book: this.name,
accounts: fromDistinctToAccounts(results),
createdAt,
expireInSec: this.expireBalanceSnapshotSec,
})
)
.catch((error) => {
console.error("medici: Couldn't do background listAccounts snapshot.", error);
});
}
const distinctResult = await transactionModel.collection.distinct(
"accounts",
{ book: this.name },
{ session: options.session }
);
const accountsSet: Set<string> = new Set();
for (const fullAccountName of distinctResult) {
const paths = fullAccountName.split(":");
let path = paths[0];
accountsSet.add(path);
for (let i = 1; i < paths.length; ++i) {
path += ":" + paths[i];
accountsSet.add(path);
}
}

return uniqueAccounts;
return Array.from(accountsSet).sort();
}
}

Expand Down
30 changes: 0 additions & 30 deletions src/models/balance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,33 +102,3 @@ export function getBestBalanceSnapshot(query: FilterQuery<IBalance>, options: IO
const key = hashKey(constructKey(book, account, { ...meta, ...extras }));
return balanceModel.collection.findOne({ key }, { sort: { _id: -1 }, ...options }) as Promise<IBalance | null>;
}

export async function snapshotListAccounts(
balanceData: { book: string; accounts: string[]; createdAt: Date; expireInSec: number },
options: IOptions = {}
): Promise<boolean> {
const rawKey = "@listAccounts:" + balanceData.book;
const key = hashKey(rawKey);
const balanceDoc = {
key,
rawKey,
book: balanceData.book,
meta: { accounts: balanceData.accounts },
createdAt: balanceData.createdAt,
expireAt: new Date(balanceData.createdAt.getTime() + balanceData.expireInSec * 1000),
};
const result = await balanceModel.collection.insertOne(balanceDoc, {
session: options.session,
writeConcern: options.session ? undefined : { w: 1, j: true }, // Ensure at least ONE node wrote to JOURNAL (disk)
forceServerObjectId: true,
});
return result.acknowledged;
}

export async function getBestListAccountsSnapshot(
query: { book: string },
options: IOptions = {}
): Promise<IBalance | null> {
const key = hashKey("@listAccounts:" + query.book);
return (await balanceModel.collection.findOne({ key }, { sort: { _id: -1 }, ...options })) as IBalance | null;
}

0 comments on commit e91723c

Please sign in to comment.