Skip to content

Commit

Permalink
feat: Take into account number of databases in an instance
Browse files Browse the repository at this point in the history
Instances configured with <1000 PUs only support 10 DBs per 100 PUs
so clamp the minimum size to the relevant number of PUs

Fixes cloudspannerecosystem#286
  • Loading branch information
nielm committed May 10, 2024
1 parent 3cbf22f commit 9f68b67
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/autoscaler-common/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const AutoscalerUnits = {
* currentSize: number,
* regional: boolean,
* currentNodes: number,
* numDatabases: number,
* }} SpannerMetadata
*/

Expand Down
15 changes: 13 additions & 2 deletions src/poller/poller-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,13 @@ async function getSpannerMetadata(projectId, spannerInstanceId, units) {

try {
const spannerInstance = spanner.instance(spannerInstanceId);
const data = await spannerInstance.getMetadata();
const metadata = data[0];

const results = await Promise.all([
spannerInstance.getDatabases(),
spannerInstance.getMetadata(),
]);
const numDatabases = results[0][0].length;
const metadata = results[1][0];
logger.debug({
message: `DisplayName: ${metadata['displayName']}`,
projectId: projectId,
Expand All @@ -294,6 +299,11 @@ async function getSpannerMetadata(projectId, spannerInstanceId, units) {
projectId: projectId,
instanceId: spannerInstanceId,
});
logger.debug({
message: `numDatabases: ${numDatabases}`,
projectId: projectId,
instanceId: spannerInstanceId,
});

/** @type {SpannerMetadata} */
const spannerMetadata = {
Expand All @@ -304,6 +314,7 @@ async function getSpannerMetadata(projectId, spannerInstanceId, units) {
regional: !!metadata['config']?.split('/')?.pop()?.startsWith('regional'),
// DEPRECATED
currentNodes: assertDefined(metadata['nodeCount']),
numDatabases: numDatabases,
};
return spannerMetadata;
} finally {
Expand Down
1 change: 1 addition & 0 deletions src/scaler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ The following is an example:
}
],
"currentSize":100,
"numDatabases": 10,
"regional":true
}
```
Expand Down
20 changes: 20 additions & 0 deletions src/scaler/scaler-core/scaling-methods/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ const RelativeToRange = {
// margin
const DEFAULT_THRESHOLD_MARGIN = 5;

// Min 10 databases per 100 processing units.
// https://cloud.google.com/spanner/quotas#database-limits
const DATABASES_PER_100_PU = 10;

const {logger} = require('../../../autoscaler-common/logger');
const {AutoscalerUnits} = require('../../../autoscaler-common/types');

/**
* @typedef {import('../../../autoscaler-common/types').AutoscalerSpanner
Expand Down Expand Up @@ -170,6 +175,21 @@ function loopThroughSpannerMetrics(spanner, getSuggestedSize) {
});

let maxSuggestedSize = spanner.minSize;

if (
spanner.units === AutoscalerUnits.PROCESSING_UNITS &&
spanner.numDatabases
) {
const minUnitsForNumDatabases =
Math.ceil(spanner.numDatabases / DATABASES_PER_100_PU) * 100;
logger.info({
message: `\tMinumum ${minUnitsForNumDatabases} ${spanner.units} required for ${spanner.numDatabases} databases`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
});
maxSuggestedSize = Math.max(maxSuggestedSize, minUnitsForNumDatabases);
}

spanner.isOverloaded = false;

for (const metric of /** @type {SpannerMetricValue[]} */ (spanner.metrics)) {
Expand Down
18 changes: 18 additions & 0 deletions src/scaler/scaler-core/test/scaling-methods/base.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,22 @@ describe('#loopThroughSpannerMetrics', () => {
spanner.metrics[0].should.have.property('margin');
spanner.metrics[1].should.have.property('margin').and.equal(99);
});

it('should clamp to a min number of PU based on numDatabases', () => {
const spanner = /** @type {AutoscalerSpanner} */ ({
units: 'PROCESSING_UNITS',
minSize: 100,
numDatabases: 55,
currentSize: 1000,
maxSize: 2000,
metrics: [
{name: 'high_priority_cpu', threshold: 65, value: 10},
{name: 'rolling_24_hr', threshold: 90, value: 10},
],
});

// Metrics suggest 100PU, but 55 db's requires 600 PU
const suggestedSize = loopThroughSpannerMetrics(spanner, () => 100);
suggestedSize.should.be.equal(600);
});
});
1 change: 1 addition & 0 deletions src/scaler/scaler-core/test/state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ const BASE_CONFIG = {
maxSize: 200,
stepSize: 10,
overloadStepSize: 10,
numDatabases: 1,
};

describe('stateFirestoreTests', () => {
Expand Down

0 comments on commit 9f68b67

Please sign in to comment.