From 3dd41026aaa91ced652b45549bf3379cf788e9d1 Mon Sep 17 00:00:00 2001 From: Metin Dumandag <29387993+mdumandag@users.noreply.github.com> Date: Tue, 21 May 2024 14:46:11 +0300 Subject: [PATCH 1/3] Improve delete namespace test This test was causing some exceptions on the server-side (that are not problematic, just polluting the logs) because it was deleting the namespace right after an insert, in almost the same time as the transient indexing is being done. This was causing us to reject transient indexing operation and log that exception in server's log. This exception is totally safe and can result in normal execution of our APIs, but this test was causing it from time to time. I have modified the test to wait for transient indexing to finish and removed the unnecessary sleep after the namespace is deleted, as the delete namespace operation is visible to the list namespaces operation immediately. --- .../namespaces/delete/index.test.ts | 7 +--- src/utils/test-utils.ts | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/commands/management/namespaces/delete/index.test.ts b/src/commands/management/namespaces/delete/index.test.ts index 2ffb200..a70b859 100644 --- a/src/commands/management/namespaces/delete/index.test.ts +++ b/src/commands/management/namespaces/delete/index.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test"; import { DeleteNamespaceCommand, ListNamespacesCommand, UpsertCommand } from "@commands/index"; -import { newHttpClient, randomID, range } from "@utils/test-utils"; -import { sleep } from "bun"; +import { awaitUntilIndexed, newHttpClient, randomID, range } from "@utils/test-utils"; const client = newHttpClient(); @@ -12,7 +11,7 @@ describe("NAMESPACES->DELETE", () => { { namespace: "test-namespace-delete" } ).exec(client); - sleep(2000); + await awaitUntilIndexed(client, "test-namespace-delete"); const namespaces = await new ListNamespacesCommand().exec(client); @@ -20,8 +19,6 @@ describe("NAMESPACES->DELETE", () => { await new DeleteNamespaceCommand("test-namespace-delete").exec(client); - sleep(2000); - const namespacesAfterDelete = await new ListNamespacesCommand().exec(client); expect(namespacesAfterDelete).not.toContain("test-namespace-delete"); diff --git a/src/utils/test-utils.ts b/src/utils/test-utils.ts index 5358829..9783e54 100644 --- a/src/utils/test-utils.ts +++ b/src/utils/test-utils.ts @@ -1,3 +1,5 @@ +import { InfoCommand } from "@commands/client"; +import { sleep } from "bun"; import { ResetCommand } from "../commands/client/reset"; import { HttpClient, RetryConfig } from "../http"; @@ -58,3 +60,38 @@ export const range = (start: number, end: number, step = 1) => { } return result; }; + +export const awaitUntilIndexed = async ( + client: HttpClient, + namespace: string | null = null, + timeoutMillis = 10_000 +) => { + const start = performance.now(); + + do { + const info = await new InfoCommand().exec(client); + + if (namespace === null) { + // check the total pending count if no namespace is provided + if (info.pendingVectorCount === 0) { + // OK, nothing more to index. + return; + } + } else { + const nsInfo = info.namespaces[namespace]; + if (nsInfo === undefined) { + throw new Error(`Index does not have a namespace called '${namespace}'`); + } + + if (nsInfo.pendingVectorCount === 0) { + // OK, nothing more to index. + return; + } + } + + // Not indexed yet, sleep a bit and check again if the timeout is not passed. + await sleep(1000); + } while (performance.now() < start + timeoutMillis); + + throw new Error(`Indexing is not completed in ${timeoutMillis} ms.`); +}; From 79f6118837e09db9ce26cfaab4b610f6c9a6f436 Mon Sep 17 00:00:00 2001 From: Metin Dumandag <29387993+mdumandag@users.noreply.github.com> Date: Tue, 21 May 2024 15:32:01 +0300 Subject: [PATCH 2/3] Improve tests by awaiting for indexing instead of arbitrary sleeps which are sometimes not awaited --- src/commands/client/fetch/index.test.ts | 5 +-- src/commands/client/info/index.test.ts | 7 ++-- src/commands/client/namespace/index.test.ts | 9 ++--- src/commands/client/query/index.test.ts | 27 +++++++------ .../management/namespaces/list/index.test.ts | 5 +-- src/utils/test-utils.ts | 38 ++++++++----------- 6 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/commands/client/fetch/index.test.ts b/src/commands/client/fetch/index.test.ts index ee17049..ebdd0b5 100644 --- a/src/commands/client/fetch/index.test.ts +++ b/src/commands/client/fetch/index.test.ts @@ -1,7 +1,6 @@ import { afterAll, describe, expect, test } from "bun:test"; import { FetchCommand, UpsertCommand } from "@commands/index"; -import { newHttpClient, randomID, range, resetIndexes } from "@utils/test-utils"; -import { sleep } from "bun"; +import { awaitUntilIndexed, newHttpClient, randomID, range, resetIndexes } from "@utils/test-utils"; import { Index } from "../../../../index"; const client = newHttpClient(); @@ -78,7 +77,7 @@ describe("FETCH", () => { await index.upsert(mockData, { namespace: "test" }); - sleep(4000); + await awaitUntilIndexed(index); const fetchWithID = await index.fetch([mockData.id], { includeMetadata: true, diff --git a/src/commands/client/info/index.test.ts b/src/commands/client/info/index.test.ts index f5762a6..62ebe2b 100644 --- a/src/commands/client/info/index.test.ts +++ b/src/commands/client/info/index.test.ts @@ -1,7 +1,6 @@ import { afterAll, describe, expect, test } from "bun:test"; import { UpsertCommand } from "@commands/index"; -import { newHttpClient, randomID, range, resetIndexes } from "@utils/test-utils"; -import { sleep } from "bun"; +import { awaitUntilIndexed, newHttpClient, randomID, range, resetIndexes } from "@utils/test-utils"; import { InfoCommand } from "."; const client = newHttpClient(); @@ -18,7 +17,9 @@ describe("INFO", () => { const payloads = randomizedData.map((data) => new UpsertCommand(data).exec(client)); await Promise.all(payloads); - await sleep(2000); + + await awaitUntilIndexed(client); + const res = await new InfoCommand().exec(client); expect(res.vectorCount).toBeGreaterThanOrEqual(vectorCount); }); diff --git a/src/commands/client/namespace/index.test.ts b/src/commands/client/namespace/index.test.ts index 420dd5d..d7b66d7 100644 --- a/src/commands/client/namespace/index.test.ts +++ b/src/commands/client/namespace/index.test.ts @@ -1,7 +1,6 @@ import { afterAll, describe, expect, test } from "bun:test"; -import { range, resetIndexes } from "@utils/test-utils"; +import { awaitUntilIndexed, range, resetIndexes } from "@utils/test-utils"; -import { sleep } from "bun"; import { Index } from "../../../../index"; describe("NAMESPACE", () => { @@ -27,7 +26,7 @@ describe("NAMESPACE", () => { metadata: { namespace: "namespace2" }, }); - sleep(10000); + await awaitUntilIndexed(index); const query1 = await namespace1.query({ vector: range(0, 384), @@ -55,7 +54,7 @@ describe("NAMESPACE", () => { metadata: { namespace: "test-namespace-reset" }, }); - sleep(5000); + await awaitUntilIndexed(index); const res = await namespace.query({ vector: range(0, 384), @@ -66,8 +65,6 @@ describe("NAMESPACE", () => { await namespace.reset(); - sleep(5000); - const res2 = await namespace.query({ vector: range(0, 384), topK: 3, diff --git a/src/commands/client/query/index.test.ts b/src/commands/client/query/index.test.ts index a618624..cd3d7b1 100644 --- a/src/commands/client/query/index.test.ts +++ b/src/commands/client/query/index.test.ts @@ -1,7 +1,6 @@ import { afterAll, describe, expect, test } from "bun:test"; import { QueryCommand, UpsertCommand } from "@commands/index"; -import { newHttpClient, range, resetIndexes } from "@utils/test-utils"; -import { sleep } from "bun"; +import { awaitUntilIndexed, newHttpClient, range, resetIndexes } from "@utils/test-utils"; const client = newHttpClient(); @@ -11,8 +10,8 @@ describe("QUERY", () => { const initialVector = range(0, 384); const initialData = { id: 33, vector: initialVector }; await new UpsertCommand(initialData).exec(client); - //This is needed for vector index insertion to happen. When run with other tests in parallel this tends to fail without sleep. But, standalone it should work without an issue. - await sleep(2000); + + await awaitUntilIndexed(client); const res = await new QueryCommand<{ hello: "World" }>({ includeVectors: true, @@ -42,8 +41,9 @@ describe("QUERY", () => { }, }; await new UpsertCommand(initialData).exec(client); - //This is needed for vector index insertion to happen. When run with other tests in parallel this tends to fail without sleep. But, standalone it should work without an issue. - await sleep(2000); + + await awaitUntilIndexed(client); + const res = await new QueryCommand<{ city: string; population: number; @@ -91,8 +91,9 @@ describe("QUERY", () => { }, ]; await new UpsertCommand(initialData).exec(client); - //This is needed for vector index insertion to happen. When run with other tests in parallel this tends to fail without sleep. But, standalone it should work without an issue. - await sleep(2000); + + await awaitUntilIndexed(client); + const res = await new QueryCommand<{ animal: string; tags: string[]; @@ -129,8 +130,9 @@ describe("QUERY", () => { metadata: { upstash: "test" }, }, ]).exec(embeddingClient); - // This is needed for vector index insertion to happen. When run with other tests in parallel this tends to fail without sleep. But, standalone it should work without an issue. - await sleep(5000); + + await awaitUntilIndexed(embeddingClient); + const res = await new QueryCommand({ data: "Test1-2-3-4-5", topK: 1, @@ -163,8 +165,9 @@ describe("QUERY", () => { metadata: { upstash: "Monster" }, }, ]).exec(embeddingClient); - // This is needed for vector index insertion to happen. When run with other tests in parallel this tends to fail without sleep. But, standalone it should work without an issue. - await sleep(5000); + + await awaitUntilIndexed(embeddingClient); + const res = await new QueryCommand({ data: "Test1-2-3-4-5", topK: 1, diff --git a/src/commands/management/namespaces/list/index.test.ts b/src/commands/management/namespaces/list/index.test.ts index 4e737a8..03e828d 100644 --- a/src/commands/management/namespaces/list/index.test.ts +++ b/src/commands/management/namespaces/list/index.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test"; import { ListNamespacesCommand, UpsertCommand } from "@commands/index"; -import { newHttpClient, randomID, range } from "@utils/test-utils"; -import { sleep } from "bun"; +import { awaitUntilIndexed, newHttpClient, randomID, range } from "@utils/test-utils"; const client = newHttpClient(); @@ -12,7 +11,7 @@ describe("NAMESPACES->LIST", () => { { namespace: "test-namespace-list" } ).exec(client); - sleep(2000); + await awaitUntilIndexed(client); const namespaces = await new ListNamespacesCommand().exec(client); diff --git a/src/utils/test-utils.ts b/src/utils/test-utils.ts index 9783e54..3562ee0 100644 --- a/src/utils/test-utils.ts +++ b/src/utils/test-utils.ts @@ -1,7 +1,8 @@ -import { InfoCommand } from "@commands/client"; import { sleep } from "bun"; +import { InfoCommand } from "../commands/client/info"; import { ResetCommand } from "../commands/client/reset"; import { HttpClient, RetryConfig } from "../http"; +import { Index } from "../vector"; export type NonArrayType = T extends Array ? U : T; @@ -61,32 +62,23 @@ export const range = (start: number, end: number, step = 1) => { return result; }; -export const awaitUntilIndexed = async ( - client: HttpClient, - namespace: string | null = null, - timeoutMillis = 10_000 -) => { +export const awaitUntilIndexed = async (client: HttpClient | Index, timeoutMillis = 10_000) => { const start = performance.now(); - do { - const info = await new InfoCommand().exec(client); + const getInfo = async () => { + if (client instanceof HttpClient) { + const cmd = new InfoCommand(); + return await cmd.exec(client); + } - if (namespace === null) { - // check the total pending count if no namespace is provided - if (info.pendingVectorCount === 0) { - // OK, nothing more to index. - return; - } - } else { - const nsInfo = info.namespaces[namespace]; - if (nsInfo === undefined) { - throw new Error(`Index does not have a namespace called '${namespace}'`); - } + return await client.info(); + }; - if (nsInfo.pendingVectorCount === 0) { - // OK, nothing more to index. - return; - } + do { + const info = await getInfo(); + if (info.pendingVectorCount === 0) { + // OK, nothing more to index. + return; } // Not indexed yet, sleep a bit and check again if the timeout is not passed. From 361cff75ebd579332644c4e2d6acd581f418c560 Mon Sep 17 00:00:00 2001 From: Metin Dumandag <29387993+mdumandag@users.noreply.github.com> Date: Tue, 21 May 2024 15:34:53 +0300 Subject: [PATCH 3/3] fix leftover parameter --- src/commands/management/namespaces/delete/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/management/namespaces/delete/index.test.ts b/src/commands/management/namespaces/delete/index.test.ts index a70b859..14d18b9 100644 --- a/src/commands/management/namespaces/delete/index.test.ts +++ b/src/commands/management/namespaces/delete/index.test.ts @@ -11,7 +11,7 @@ describe("NAMESPACES->DELETE", () => { { namespace: "test-namespace-delete" } ).exec(client); - await awaitUntilIndexed(client, "test-namespace-delete"); + await awaitUntilIndexed(client); const namespaces = await new ListNamespacesCommand().exec(client);