Skip to content

tests(bigtable): fix resource leaks in bigtable system tests #9030

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
feywind wants to merge 6 commits into
base: main
Choose a base branch
from
Open

tests(bigtable): fix resource leaks in bigtable system tests #9030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e80735f
tests: clean up some resource leaks for system tests
feywind Jul 28, 2026
55449ee
tests: lower some resource usage in system tests, and catch more leaks
feywind Jul 29, 2026
13e1574
tests: more resource leak cleanup
feywind Jul 29, 2026
028fc61
chore: update for Gemini CRs
feywind Jul 30, 2026
dc9a54f
tests: shorten instance name to fix a failure (max 36 chars)
feywind Jul 30, 2026
872261a
tests: add client-side metrics retries
feywind Jul 30, 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
5 changes: 3 additions & 2 deletions handwritten/bigtable/system-test/app-profile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

import {describe, it, before, after} from 'mocha';
import {generateId} from './common';
import {generateId, reapInstances} from './common';
import {AppProfileOptions, Bigtable, Instance} from '../src';
import {AppProfile} from '../src';
import assert = require('assert');
Expand All @@ -38,6 +38,7 @@ describe('📦 App Profile', () => {
}

before(async () => {
await reapInstances(bigtable);
// Creates an instance with clusters
const instanceClusters = [
'us-east1-c',
Expand All @@ -60,7 +61,7 @@ describe('📦 App Profile', () => {
};
}),
labels: {
time_created: Date.now(),
time_created: String(Date.now()),
},
});
await operation.promise();
Expand Down
89 changes: 23 additions & 66 deletions handwritten/bigtable/system-test/bigtable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {Family} from '../src/family.js';
import {Row} from '../src/row.js';
import {Table} from '../src/table.js';
import {RawFilter} from '../src/filter';
import {generateId, PREFIX} from './common';
import {generateId, reapBackups, reapInstances} from './common';
import {BigtableTableAdminClient} from '../src/v2';
import {ServiceError} from 'google-gax';
import {BigtableDate, QueryResultRow} from '../src/execute-query/values';
Expand All @@ -56,73 +56,32 @@ describe('Bigtable', () => {
const CLUSTER_ID = generateId('cluster');
const CLUSTER_ID_HDD = generateId('cluster');

async function reapBackups(instance: Instance) {
try {
const [backups] = await instance.getBackups();
for (const backup of backups) {
try {
await backup.delete({timeout: 50 * 1000});
} catch (e) {
console.log(`Error deleting backup: ${backup.id}: ${e}`);
}
}
} catch (e) {
console.error(`Error listing backups from ${instance.name}: ${e}`);
}
}

async function reapInstances() {
const [instances] = await bigtable.getInstances();
const testInstances = instances
.filter(i => i.id.match(PREFIX))
.filter(i => {
const timeCreated = i.metadata!.labels!.time_created as {} as Date;
// Only delete stale resources.
const oneHourAgo = new Date(Date.now() - 3600000);
return !timeCreated || timeCreated <= oneHourAgo;
});
// need to delete backups first due to instance deletion precondition
const deleteBackupPromises = testInstances.map(instance =>
reapBackups(instance),
);
for (const backupPromise of deleteBackupPromises) {
await backupPromise;
}
for (const instance of testInstances) {
try {
await instance.delete();
} catch (e) {
console.log(`Error deleting instance: ${instance.id}`);
}
}
}

before(async () => {
await reapInstances();
await reapInstances(bigtable);
const [, operation] = await INSTANCE.create({
clusters: [
{
id: CLUSTER_ID,
location: 'us-central2-c',
nodes: 3,
nodes: 1,
storage: 'ssd',
},
],
labels: {
time_created: Date.now(),
time_created: String(Date.now()),
},
});
const [, operationHDD] = await INSTANCE_HDD.create({
clusters: [
{
id: CLUSTER_ID_HDD,
location: 'us-central2-c',
nodes: 3,
nodes: 1,
storage: 'hdd',
},
],
labels: {
time_created: Date.now(),
time_created: String(Date.now()),
},
});

Expand All @@ -140,22 +99,20 @@ describe('Bigtable', () => {
});

after(async () => {
const q = [];
const instances = [INSTANCE, DIFF_INSTANCE, CMEK_INSTANCE, INSTANCE_HDD];

// need to delete backups first due to instance deletion precondition
await Promise.all(instances.map(instance => reapBackups(instance)));
await Promise.all(
instances.map(instance => {
q.push(async () => {
try {
await instance.delete();
} catch (e) {
console.log(`Error deleting instance: ${instance.id}`);
}
});
instances.map(async instance => {
try {
await instance.delete();
} catch (e) {
console.log(`Error deleting instance: ${instance.id}`);
}
}),
);
await reapInstances(bigtable);
});

describe('instances', () => {
Expand Down Expand Up @@ -215,11 +172,11 @@ describe('Bigtable', () => {
{
id: clusteId,
location: 'us-central2-c',
nodes: 3,
nodes: 1,
},
],
labels: {
time_created: Date.now(),
time_created: String(Date.now()),
},
});
await operation.promise();
Expand Down Expand Up @@ -268,12 +225,12 @@ describe('Bigtable', () => {
{
id: CMEK_CLUSTER.id,
location: 'us-central2-a',
nodes: 3,
nodes: 1,
key: kmsKeyName,
},
],
labels: {
time_created: Date.now(),
time_created: String(Date.now()),
},
});
await operation.promise();
Expand All @@ -300,7 +257,7 @@ describe('Bigtable', () => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [_, operation] = await cluster.create({
location: 'us-central2-b',
nodes: 3,
nodes: 1,
key: kmsKeyName,
});
await operation.promise();
Expand All @@ -316,7 +273,7 @@ describe('Bigtable', () => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [_, operation] = await cluster.create({
location: 'us-central2-b',
nodes: 3,
nodes: 1,
});
await operation.promise();
throw new Error('Cluster creation should not have succeeded');
Expand Down Expand Up @@ -432,7 +389,7 @@ describe('Bigtable', () => {

it('should update a cluster', async () => {
const metadata = {
nodes: 4,
nodes: 2,
};
const [operation] = await CLUSTER.setMetadata(metadata);
await operation.promise();
Expand Down Expand Up @@ -1716,7 +1673,7 @@ describe('Bigtable', () => {
clusters: [
{
id: destinationClusterId,
nodes: 3,
nodes: 1,
location: 'us-central2-d',
storage: 'ssd',
},
Expand Down Expand Up @@ -1760,7 +1717,7 @@ describe('Bigtable', () => {
destinationClusterId,
).create({
location: 'us-central2-b',
nodes: 3,
nodes: 1,
});
await operation.promise();
}
Expand Down Expand Up @@ -1805,7 +1762,7 @@ describe('Bigtable', () => {
clusters: [
{
id: destinationClusterId,
nodes: 3,
nodes: 1,
location: 'us-central2-d',
storage: 'ssd',
},
Expand Down
Loading
Loading