Ensure current connection info remains available during ZDT

[enocom]

During Zonal Down Time, the Connector will experience a short period of connection failures. Currently, we force refresh on each connection failure. This feature request is to ensure that if the refresh is rate limited, the current connection info remains available to support fast re-connections.

[enocom]

This is probably just a matter of updating refresh to not block on the next refresh operation here:

cloud-sql-nodejs-connector/src/cloud-sql-instance.ts

Line 111 in 290d741

await this.next;

EDIT fixed my bad links

[ruyadorno]

I believe this was handled in #219, more specifically this line should take care of refreshing automatically and still apply rate limits on connection failures:

cloud-sql-nodejs-connector/src/cloud-sql-instance.ts

Line 161 in 290d741

this.scheduleRefresh(0);

While current connection info should also remain available to support new connections, given that we only update values in a CloudSQLInstance once a refresh has successfully completed:

cloud-sql-nodejs-connector/src/cloud-sql-instance.ts

Lines 141 to 143 in 290d741

	// In case the performRefresh method succeeded
	// then we go ahead and update values
	this.updateValues(nextValues);

@jackwotherspoon I know you helped me review some of this logic and I would love to have your input here. Should we close this issue?

[enocom]

What I'm wondering about is the significance of this line in particular. By awaiting the next refresh, what does that do for other connection attempts?

cloud-sql-nodejs-connector/src/cloud-sql-instance.ts

Line 111 in 290d741

await this.next;

[ruyadorno]

oh, this never really blocks anything (other than in tests) since the only place forceRefresh is called from is this callback handler that doesn't block any other logic:

cloud-sql-nodejs-connector/src/connector.ts

Lines 220 to 222 in 290d741

	tlsSocket.once('error', async () => {
	await cloudSqlInstance.forceRefresh();
	});

Connection attempts from the driver would not be prevented from that. Looking at this particularly part of the code, I believe I should remove the async/await since it server no purpose here in this callback.

[enocom]

Got it. In that case, I agree -- we fixed this in #219.

[ruyadorno]

Sounds good 👍 thanks @enocom!