Skip to content

make read txn timeout configurable and set default to 1min #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ldt1996 wants to merge 4 commits into
base: main
Choose a base branch
from
+69 −4
Open

make read txn timeout configurable and set default to 1min #304

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1610845
make read txn timeout configurable and set default to 1min
ldt1996 Mar 31, 2026
2527d44
fix formatting
ldt1996 Mar 31, 2026
93cafaa
remove clamping + increase exp time
ldt1996 Apr 12, 2026
efa7ba4
Merge branch 'main' into read_txn_expiration
ldt1996 Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions resources/RecordEncoder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { blobsWereEncoded, decodeFromDatabase, deleteBlobsInObject, encodeBlobsW
import { recordAction } from './analytics/write.ts';
import { RocksDatabase } from '@harperfast/rocksdb-js';
import { when } from '../utility/when.ts';
import { CONFIG_PARAMS } from '../utility/hdbTerms.ts';
import * as envMngr from '../utility/environment/environmentManager.js';
export type Entry = {
key: any;
value: any;
Expand Down Expand Up @@ -485,16 +487,18 @@ export function handleLocalTimeForGets(store, rootStore) {
return store;
}
const trackedTxns: WeakRef<any>[] = [];
setInterval(() => {
const configValue = envMngr.get(CONFIG_PARAMS.STORAGE_MAXREADTRANSACTIONOPENTIME) ?? 60000;
let READ_TXN_TIMEOUT_TICKS = Math.round(Math.min(Math.max(configValue, 15000), 300000) / 15000); // clamp between 15s and 5min
Copy link
Copy Markdown
Member

@kriszyp kriszyp Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we actually need to clamp this. I know I had mentioned concerns about excessively long timeouts here, but that was more of a consideration for those setting the value, but I trust that we can set the value properly. And I think that a default of one minute is too short. I believe the previous setting was 15 minutes, right? Maybe the default should be 5 minutes?

Copy link
Copy Markdown
Contributor Author

@ldt1996 ldt1996 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it is finally 93cafaa

export function checkReadTxnTimeouts() {
for (let i = 0; i < trackedTxns.length; i++) {
const txn = trackedTxns[i].deref();
if (!txn || txn.isDone || txn.isCommitted) trackedTxns.splice(i--, 1);
else if (txn.notCurrent) {
if (txn.openTimer) {
if (txn.openTimer > 3) {
if (txn.openTimer > 60) {
if (txn.openTimer > READ_TXN_TIMEOUT_TICKS) {
harperLogger.error(
'Read transaction detected that has been open too long (over 15 minutes), ending transaction',
`Read transaction detected that has been open too long (over ${Math.round(READ_TXN_TIMEOUT_TICKS * 15)} seconds), ending transaction`,
txn
);
txn.done();
Expand All @@ -508,7 +512,12 @@ setInterval(() => {
} else txn.openTimer = 1;
}
}
}, 15000).unref();
}
setInterval(checkReadTxnTimeouts, 15000).unref();
export function setReadTxnExpiration(ms: number) {
READ_TXN_TIMEOUT_TICKS = Math.round(Math.min(Math.max(ms, 15000), 300000) / 15000);
}

export function recordUpdater(store, tableId, auditStore) {
return function (
id,
Expand Down
55 changes: 55 additions & 0 deletions unitTests/resources/txn-tracking.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const assert = require('assert');
const { setupTestDBPath } = require('../testUtils');
const { setTxnExpiration } = require('#src/resources/DatabaseTransaction');
const { setTxnExpiration: setLMDBTxnExpiration } = require('#src/resources/LMDBTransaction');
const { setReadTxnExpiration, checkReadTxnTimeouts } = require('#src/resources/RecordEncoder');
const { setMainIsWorker } = require('#js/server/threads/manageThreads');
const { table } = require('#src/resources/databases');
const { setTimeout: delay } = require('node:timers/promises');
Expand Down Expand Up @@ -54,3 +55,57 @@ describe('Txn Expiration', () => {
setTxnExpiration(30000);
});
});

describe('Read Txn Expiration', () => {
let SlowReadResource;
before(async function () {
setupTestDBPath();
setMainIsWorker(true);
let BasicTable = table({
table: 'ReadTxnTable',
database: 'test',
attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }],
});
SlowReadResource = class extends BasicTable {
async get(query) {
const result = super.get(query);
await delay(50);
return result;
}
};
});

it('Read txn will be ended after timeout', async function () {
await SlowReadResource.put(1, { name: 'one' });

// set timeout to minimum, 15s = 1 tick, openTimer > 1 means txn is expired
setReadTxnExpiration(15000);

const readPromise = SlowReadResource.get(1);
await delay(20);

// simulate timer ticks
checkReadTxnTimeouts();
checkReadTxnTimeouts();

await readPromise;
});

it('Read txn below threshold is not expired', async function () {
setReadTxnExpiration(60000);

await SlowReadResource.put(2, { name: 'two' });
const readPromise = SlowReadResource.get(2);
await delay(20);

// only 2 ticks
checkReadTxnTimeouts();

const result = await readPromise;
assert.equal(result.name, 'two');
});

after(function () {
setReadTxnExpiration(60000);
});
});
1 change: 1 addition & 0 deletions utility/hdbTerms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ export const CONFIG_PARAMS = {
STORAGE_ENCRYPTION: 'storage_encryption',
STORAGE_MAXTRANSACTIONQUEUETIME: 'storage_maxTransactionQueueTime',
STORAGE_MAXTRANSACTIONOPENTIME: 'storage_maxTransactionOpenTime',
STORAGE_MAXREADTRANSACTIONOPENTIME: 'storage_maxReadTransactionOpenTime',
STORAGE_DEBUGLONGTRANSACTIONS: 'storage_debugLongTransactions',
STORAGE_PATH: 'storage_path',
STORAGE_BLOBPATHS: 'storage_blobPaths',
Expand Down
Loading