Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions common/api/core-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,7 @@ export interface DbCloudContainerInfo {
// @internal (undocumented)
export interface DbQueryConfig {
autoShutdownWhenIdleForSeconds?: number;
// @deprecated (undocumented)
doNotUsePrimaryConnToPrepare?: boolean;
// (undocumented)
globalQuota?: QueryQuota;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@itwin/core-backend",
"comment": "Added regression coverage that the deprecated `doNotUsePrimaryConnToPrepare` concurrent query config option is a no-op that still round-trips for backward-compatible serialization.",
"type": "none"
}
],
"packageName": "@itwin/core-backend",
"email": "khanaffan@users.noreply.github.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-backend",
"comment": "Add performance test for concurrent query",
"type": "none"
}
],
"packageName": "@itwin/core-backend"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@itwin/core-common",
"comment": "Deprecated the no-op `doNotUsePrimaryConnToPrepare` concurrent query config option. Worker connections now always prepare against a shared, dedicated schema-source connection (falling back to their own connection) to avoid a deadlock with the primary connection; the option is retained only for backward-compatible config serialization and no longer affects behavior.",
"type": "none"
}
],
"packageName": "@itwin/core-common",
"email": "khanaffan@users.noreply.github.com"
}
6 changes: 4 additions & 2 deletions core/backend/src/test/ecdb/ConcurrentQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ describe("ConcurrentQuery", () => {
expect(resp.stats.cpuTime).to.be.closeTo(1000970, 500000);
expect(resp.stats.totalTime).to.be.closeTo(1001, 100);
expect(resp.stats.memUsed).to.be.closeTo(2, 3);
expect(resp.stats.prepareTime).to.be.closeTo(0, 2);
// prepareTime varies on CI: the first query prepares against the (cold) shared schema-source
// connection, so assert only that it is negligible relative to the ~1000ms execution rather than
// pinning it to a tight tolerance (which flakes -- e.g. observed 7ms against a 0 +/- 4 bound).
expect(resp.stats.prepareTime).to.be.lessThan(100);
db.close();
});

Expand Down Expand Up @@ -184,7 +187,6 @@ describe("ConcurrentQuery", () => {
const config = {
requestQueueSize: 1000,
statementCacheSizePerWorker: 1, // Force frequent prepare calls
doNotUsePrimaryConnToPrepare: false, // Force primary connection usage
};

// Reset configuration
Expand Down
2 changes: 1 addition & 1 deletion core/backend/src/test/ecdb/ECDb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe("ECDb", () => {
alias: "source",
fileName: path.join(outDir, "source_file.ecdb"),
profile: "SQLite"
}
},
]);
testECDb1.detachDb("source");
expect(await runDbListPragmaCCQ(testECDb1)).deep.equals([
Expand Down
10 changes: 8 additions & 2 deletions core/backend/src/test/ecdb/ECSqlStatement.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3431,8 +3431,14 @@ describe("ECSqlStatement", () => {
parentHasChildrenClassId = reader.current.ECInstanceId;
assert.isTrue(Id64.isValidId64(parentHasChildrenClassId));

// When the ECSql insert validation is set to true, the invalid relClassId is detected and an error is thrown.
assert.isTrue(await (ecdb.createQueryReader("PRAGMA validate_ecsql_writes=true")).step());
// Enable ECSql write-value validation so invalid relClassIds are detected and an error is thrown.
// This must be set synchronously on the primary connection (the connection on which the write
// statements below are prepared and validated). It cannot be set via `createQueryReader`, whose
// concurrent-query path executes the pragma on a separate worker connection that does not affect
// the primary connection's write validation.
ecdb.withWriteStatement("PRAGMA validate_ecsql_writes=true", (stmt: ECSqlWriteStatement) => {
Comment thread
aruniverse marked this conversation as resolved.
assert.equal(stmt.step(), DbResult.BE_SQLITE_ROW);
});

reader = ecdb.createQueryReader("SELECT ECInstanceId FROM meta.ECClassDef WHERE Name='ParentHasChildren'");
assert.isTrue(await reader.step());
Expand Down
7 changes: 6 additions & 1 deletion core/common/src/ConcurrentQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,12 @@ export interface DbQueryConfig {
requestQueueSize?: number;
/** Number of worker thread, default to 4 */
workerThreads?: number;
/** Use thread connection to prepare the statement */
/**
* @deprecated in 5.11.0. No longer used. Worker connections now prepare statements against a shared, dedicated
* schema-source connection (falling back to their own connection) to avoid an AB-BA lock-ordering deadlock with
* the primary connection. The value is retained only for backward-compatible config (de)serialization; setting it
* has no effect on behavior.
*/
doNotUsePrimaryConnToPrepare?: boolean;
/** After no activity for given time concurrent query will automatically shutdown */
autoShutdownWhenIdleForSeconds?: number;
Expand Down
1 change: 1 addition & 0 deletions full-stack-tests/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"perftest:schemaloader": "npm run -s perftest:pre && mocha --timeout=999999999 \"./lib/cjs/perftest/SchemaLoader.test.js\"",
"perftest:ecSqlRowPerformance": "npm run -s perftest:pre && mocha --timeout=999999999 \"./lib/cjs/perftest/ECSqlRow.test.js\"",
"perftest:readQueryPerformance": "npm run -s perftest:pre && mocha --timeout=999999999 \"./lib/cjs/perftest/ReadQueryPerf.test.js\"",
"perftest:ecSqlReaderConcurrent": "npm run -s perftest:pre && mocha --timeout=999999999 \"./lib/cjs/perftest/ECSqlReaderConcurrentPerf.test.js\"",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to run them as part of some pipeline as well? Also, do we want to generate an artifact to record the performance numbers so that we can see how performance number change in future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. It be great to track performance.

"perftest:metadataPerformance": "npm run -s perftest:pre && mocha --timeout=999999999 --grep PerformanceElementGetMetadata \"./lib/cjs/perftest/ElementCRUD.test.js\"",
"perftest:schemaContextPerformance": "npm run -s perftest:pre && mocha --timeout=999999999 \"./lib/cjs/perftest/SchemaContextIModelDb.test.js\"",
"perftest:sqliteChangesetReaderAndChangesetECAdaptorPerformance": "npm run -s perftest:pre && mocha --timeout=999999999 --grep SqliteChangesetReaderAndChangesetECAdaptorAPI \"./lib/cjs/perftest/SQliteChangesetReaderAndChangesetECAdaptor.test.js\"",
Expand Down
Loading
Loading