Skip to content
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.

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

Spanner clients hangs for a few seconds after running a query #1324

Closed
gsouf opened this issue Mar 25, 2021 · 5 comments · Fixed by #1327
Closed

Spanner clients hangs for a few seconds after running a query #1324

gsouf opened this issue Mar 25, 2021 · 5 comments · Fixed by #1327
Assignees
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@gsouf
Copy link

gsouf commented Mar 25, 2021

Environment details

  • OS: linux
  • Node.js version: 10
  • npm version: 7
  • @google-cloud/spanner version: 5.5.0

Steps to reproduce

  1. Create the following javascript file in test.js
const Spanner = require("@google-cloud/spanner").Spanner;

const projectId = "projectid";
const instance = "instance";
const database = "database";
process.env.SPANNER_EMULATOR_HOST = "localhost:9010";

const spanner = new Spanner({
    projectId: projectId,
});
const spannerInstance = spanner.instance(instance);
const spannerDatabase = spannerInstance.database(database, {
    acquireTimeout: 5000,
});

async function run() {
    try {
        console.time("run and close");
        await spannerDatabase.run({
            sql: `SELECT 1`,
        });

        await spannerDatabase.close();
        console.timeEnd("run and close");
    } catch (e) {
        console.error("caught", e);
    }
}

run();
  1. run time node test.js

Output:

$ time node ./test.js 
run and close: 142.309ms

real    0m5.420s
user    0m0.575s
sys     0m0.066s

As you can see it hangs after the query has run and it takes 5 seconds to finish the process.

The same without a the sql query spannerDatabase.run will finish almost immediately.

I only tried with spanner emulator.

Thanks

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Mar 25, 2021
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Mar 27, 2021
@skuruppu skuruppu added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels Mar 29, 2021
@skuruppu
Copy link
Contributor

@olavloite if you could please take a look at this one, that would be awesome.

olavloite added a commit to olavloite/nodejs-spanner that referenced this issue Mar 29, 2021
If an acquire timeout had been set for the session pool, the timeout handler
would not be cleared once a session was successfully returned, or when the
getSession method would fail because of another error. That could cause an
application running for at most acquireTimeout milliseconds if the last action
the application did was to acquire a session.

Fixes googleapis#1324
@olavloite
Copy link
Contributor

This problem is caused by the acquireTimeout: 5000 setting. This will cause the client library to use a 5 second timeout whenever a session is acquired from the pool. The timeout handler is however not cleared when a session is returned to the application.

The workaround in version 5.6.0 and earlier is to not set an acquireTimeout. A fix is offered in #1327

skuruppu added a commit that referenced this issue Mar 30, 2021
If an acquire timeout had been set for the session pool, the timeout handler
would not be cleared once a session was successfully returned, or when the
getSession method would fail because of another error. That could cause an
application running for at most acquireTimeout milliseconds if the last action
the application did was to acquire a session.

Fixes #1324

Co-authored-by: skuruppu <[email protected]>
@gsouf
Copy link
Author

gsouf commented Mar 30, 2021

thank you @olavloite @skuruppu

Is that going to be released with version 5.7 ? When can we expect it to be bumped?

@olavloite
Copy link
Contributor

Is that going to be released with version 5.7 ? When can we expect it to be bumped?

It should be included in 5.6.1 and higher: #1328

@skuruppu
Copy link
Contributor

I will merge the release PR shortly. Hopefully it'll be done by tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants