Skip to content

Commit

Permalink
fix: db caching won't work with replication enabled (#7694)
Browse files Browse the repository at this point in the history
* FIX Insert cache only on master connection

close #5919

* FIX release queryRunner created
  • Loading branch information
madeindjs committed Nov 16, 2021
1 parent 29cb891 commit 2d0abe7
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 3 deletions.
10 changes: 9 additions & 1 deletion src/cache/DbQueryResultCache.ts
Expand Up @@ -158,7 +158,11 @@ export class DbQueryResultCache implements QueryResultCache {
* Stores given query result in the cache.
*/
async storeInCache(options: QueryResultCacheOptions, savedCache: QueryResultCacheOptions|undefined, queryRunner?: QueryRunner): Promise<void> {
queryRunner = this.getQueryRunner(queryRunner);
const shouldCreateQueryRunner = queryRunner === undefined || queryRunner?.getReplicationMode() === "slave";

if (queryRunner === undefined || shouldCreateQueryRunner) {
queryRunner = this.connection.createQueryRunner("master");
}

let insertedValues: ObjectLiteral = options;
if (this.connection.driver instanceof SqlServerDriver) { // todo: bad abstraction, re-implement this part, probably better if we create an entity metadata for cache table
Expand Down Expand Up @@ -203,6 +207,10 @@ export class DbQueryResultCache implements QueryResultCache {
.values(insertedValues)
.execute();
}

if (shouldCreateQueryRunner) {
await queryRunner.release();
}
}

/**
Expand Down
9 changes: 7 additions & 2 deletions src/driver/mongodb/MongoQueryRunner.ts
Expand Up @@ -46,8 +46,9 @@ import {TableUnique} from "../../schema-builder/table/TableUnique";
import {Broadcaster} from "../../subscriber/Broadcaster";
import {TableCheck} from "../../schema-builder/table/TableCheck";
import {TableExclusion} from "../../schema-builder/table/TableExclusion";
import { TypeORMError } from "../../error";

import {TypeORMError} from "../../error";
import {ReplicationMode} from "../types/ReplicationMode";

/**
* Runs queries on a single MongoDB connection.
*/
Expand Down Expand Up @@ -507,6 +508,10 @@ export class MongoQueryRunner implements QueryRunner {
throw new TypeORMError(`Schema update queries are not supported by MongoDB driver.`);
}

getReplicationMode(): ReplicationMode {
return 'master';
}

/**
* Checks if database with the given name exist.
*/
Expand Down
4 changes: 4 additions & 0 deletions src/query-runner/BaseQueryRunner.ts
Expand Up @@ -203,6 +203,10 @@ export abstract class BaseQueryRunner {
}
}

getReplicationMode(): ReplicationMode {
return this.mode;
}

// -------------------------------------------------------------------------
// Protected Methods
// -------------------------------------------------------------------------
Expand Down
6 changes: 6 additions & 0 deletions src/query-runner/QueryRunner.ts
Expand Up @@ -14,6 +14,7 @@ import {TableCheck} from "../schema-builder/table/TableCheck";
import {IsolationLevel} from "../driver/types/IsolationLevel";
import {TableExclusion} from "../schema-builder/table/TableExclusion";
import {QueryResult} from "./QueryResult";
import {ReplicationMode} from "../driver/types/ReplicationMode";

/**
* Runs queries on a single database connection.
Expand Down Expand Up @@ -149,6 +150,11 @@ export interface QueryRunner {
*/
getViews(viewPaths?: string[]): Promise<View[]>;

/**
* Returns replication mode (ex: `master` or `slave`).
*/
getReplicationMode(): ReplicationMode;

/**
* Checks if a database with the given name exist.
*/
Expand Down
10 changes: 10 additions & 0 deletions test/github-issues/5919/entities.ts
@@ -0,0 +1,10 @@
import { Column, Entity, PrimaryGeneratedColumn } from "../../../src";

@Entity()
export class Comment {
@PrimaryGeneratedColumn()
id: number;

@Column()
text: string;
}
107 changes: 107 additions & 0 deletions test/github-issues/5919/issue-6399.ts
@@ -0,0 +1,107 @@
import { expect } from "chai";
import Sinon from "sinon";
import { Connection } from "../../../src";
import {
closeTestingConnections,
createTestingConnections,
reloadTestingDatabases,
} from "../../utils/test-utils";
import { Comment } from "./entities";

describe("github issues > #5919 Caching won't work with replication enabled", () => {
let connections: Connection[];

beforeEach(async () => {
connections = await createTestingConnections({
entities: [Comment],
schemaCreate: true,
dropSchema: true,
cache: true,
enabledDrivers: ["postgres"],
});
await reloadTestingDatabases(connections);
});
afterEach(() => closeTestingConnections(connections));

it("should not another queryRunner for cache with a given masterQueryRunner", () =>
Promise.all(
connections.map(async (connection) => {
const comment1 = new Comment();
comment1.text = "tata";
await connection.manager.save(comment1);

const masterQueryRunner = connection.createQueryRunner(
"master"
);
const createQueryRunnerSpy = Sinon.spy(
connection,
"createQueryRunner"
);

const results1 = await connection
.createQueryBuilder()
.from(Comment, "c")
.cache(true)
.setQueryRunner(masterQueryRunner)
.getRawMany();

expect(results1.length).eq(1);

expect(createQueryRunnerSpy.notCalled);

// add another one and ensure cache works
const comment2 = new Comment();
comment2.text = "tata";
await connection.manager.save(comment2);

const results2 = await connection
.createQueryBuilder()
.from(Comment, "c")
.cache(true)
.setQueryRunner(masterQueryRunner)
.getRawMany();

expect(results2.length).eq(1);
})
));

it("should create another queryRunner for cache with a given slaveQueryRunner", () =>
Promise.all(
connections.map(async (connection) => {
const comment1 = new Comment();
comment1.text = "tata";
await connection.manager.save(comment1);

const slaveQueryRunner = connection.createQueryRunner("slave");
const createQueryRunnerSpy = Sinon.spy(
connection,
"createQueryRunner"
);

const results1 = await connection
.createQueryBuilder()
.from(Comment, "c")
.cache(true)
.setQueryRunner(slaveQueryRunner)
.getRawMany();

expect(results1.length).eq(1);

expect(createQueryRunnerSpy.calledOnce);

// add another one and ensure cache works
const comment2 = new Comment();
comment2.text = "tata";
await connection.manager.save(comment2);

const results2 = await connection
.createQueryBuilder()
.from(Comment, "c")
.cache(true)
.setQueryRunner(slaveQueryRunner)
.getRawMany();

expect(results2.length).eq(1);
})
));
});

0 comments on commit 2d0abe7

Please sign in to comment.