fix: avoid scheduling an instance refresh if the context is done

[jomaresch]

Hello,

I discovered that closing the instance will cause an infinite loop in the scheduleRefresh function.
Closing the Instance will cancel the context, which causes an error in the Limiter:

alloydb-go-connector/internal/alloydb/instance.go

Lines 263 to 267 in ff5ca35

	ctx, cancel := context.WithTimeout(i.ctx, i.refreshTimeout)
	defer cancel()
	err := i.l.Wait(ctx)
	if err != nil {

Every error here will trigger a new refresh with 0 sec delay:

alloydb-go-connector/internal/alloydb/instance.go

Lines 283 to 286 in ff5ca35

	// if failed, scheduled the next refresh immediately
	if r.err != nil {
	i.next = i.scheduleRefresh(0)
	// If the latest result is bad, avoid replacing the

This refresh will also fail because of the canceled context.
The result is high resource consumption, once the instance has been closed, because of this infinite loop.

As a fix I propose to check the context earlier and don't schedule a new refresh once the context is closed.

I didn't write a test, because i did't find a way to properly test the absence of the loop. But I'm open for suggestions.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[conventional-commit-lint-gcf]

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

[enocom]

Nice find @jomaresch. If you can sign the CLA, we're happy to look at this. Otherwise, we can fix it in a separate PR.

[jomaresch]

@enocom I already signed the CLA. The failing check, which indicated the missing CLA, are also gone.
Am I missing anything?

[enocom]

Looking at this closely, I see there's a deeper problem. Yes, we need to move the context check up as you've done here, but we also need to stop the timers in Close. We probably also need to add a context check to ForceRefresh.

Would you be interested in landing this change, or prefer that I take it over?

See #493

[enocom]

Added the timer change and moved the context check up to the top of the function.

Thanks for sending this! We'll get it into the next release.