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

SNOW-1805253: Connection Pool #961

Closed
siirius99 opened this issue Nov 14, 2024 · 4 comments
Closed

SNOW-1805253: Connection Pool #961

siirius99 opened this issue Nov 14, 2024 · 4 comments
Assignees
Labels
question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team

Comments

@siirius99
Copy link

siirius99 commented Nov 14, 2024

  1. What version of NodeJS driver are you using?
    1.10.0

  2. What operating system and processor architecture are you using?
    Amazon Linux 2023

  3. What version of NodeJS are you using?
    (node --version and npm --version)
    16.13.0

  4. What are the component versions in the environment (npm list)?

  5. What did you do?
    I’ve been working on setting up a connection pool for Snowflake in Node.js (v16) to optimize query handling and manage
    resources efficiently. Below is the approach I took, including a custom pool configuration with the Snowflake SDK version 1.10.0.

  6. What did you expect to see?
    With this setup, I’m occasionally seeing issues where queries hang without completion . If I recreate the pool, it works again.
    This suggests there may be a problem with connection handling in the pool after prolonged use or high concurrency.

  7. Can you collect debug logs?
    No

public create = async (): Promise<any> => {
    try {
        const poolParameters = {
            username: 'your_username',
            clientSessionKeepAlive: true,
            clientSessionKeepAliveHeartbeatFrequency: 900,
            account: 'your_account',
            privateKey: 'your_private_key',
            authenticator: 'SNOWFLAKE_JWT',
        };
        return snowflake.createPool(poolParameters, {
            max: 80,  // Maximum of 80 connections in the pool
            min: 8,   // Minimum of 8 connections in the pool
        });
    } catch (error) {
        throw new Error(`Failed to activate Snowflake pool: ${error.message}`);
    }
};

const executeQuery = async (
    connectionPool: any,
    query: { text: string; values?: any[] }
): Promise<any> => {
    let connection = null;
    try {
        const result = [];
        connection = await connectionPool.acquire();

        const statement = await connection.execute({
            sqlText: query.text,
            binds: query.values ?? [],
        });

        return new Promise((resolve, reject) => {
            const stream = statement.streamRows();
            stream.on('data', (row: Record<string, any>) => result.push(row));
            stream.on('end', () => resolve(result));
            stream.on('error', (error) => reject(error));
        });
    } catch (error) {
        throw new Error(`Query execution failed: ${error.message}`);
    } finally {
        if (connection) {
            connectionPool.release(connection);
        }
    }
};

@siirius99 siirius99 added the bug Something isn't working label Nov 14, 2024
@github-actions github-actions bot changed the title Connection Pool SNOW-1805253: Connection Pool Nov 14, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Nov 15, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage Issue is under initial triage status-information_needed Additional information is required from the reporter and removed bug Something isn't working labels Nov 15, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

hi - thanks for reporting this. Before analyzing the issue deeper, could you please try using 1.15.0 version of the driver, if you have no objections against using the newest one ?

second, we're deprecating support for node:16 in the very near future - actually this runtime is already EOL just for some reason we claimed support for it. It will change in the near future.

Recommended to try on node:18 or later versions, and the latest driver version as mentioned.

if the issue still reproduces for you , I'm afraid we cannot avoid collecting and sharing DEBUG level logs, or providing a full reproduction which when run on our side, reproduces the same issue you're seeing.
(The code snippet you shared doesn't seem to reflect any kind of concurrency, or prolonged usage, which you mentioned to be contributing factors to this issue.)

Let us know how the testing went please.

@siirius99
Copy link
Author

siirius99 commented Nov 15, 2024

Hi @sfc-gh-dszmolka ,
Thanks for your suggestions.
I’ll test the issue with the latest driver version (1.15.0) and Node:18 as recommended and will update you with the results.
Could you also let me know if any changes to my code are necessary, or if it should work as is?

@sfc-gh-dszmolka
Copy link
Collaborator

the code looks fine at a quick glance, but depending on the environment you're running the application in, clientSessionKeepAliveHeartbeatFrequency: 900, might not be enough if there's a network node which tears down an idle connection unilaterally , without telling the client, sooner than 15m of idleness (with this setting, which is the most frequent setting possible from this setting, keepalives are sent every 15m. That might not be frequent enough)

so instead of clientSessionKeepAlive / clientSessionKeepAliveHeartbeatFrequency, you might want to consider setting up the pool in a way that it detects and gracefully reaps and reopens idle connections - before any firewall etc. could do.
Examples are in progress of making their way into the Snowflake documentation, until the docs team is done with the changes, you can look at examples like this comment

@sfc-gh-dszmolka
Copy link
Collaborator

closing this one as there's been no activity for a while.

@sfc-gh-dszmolka sfc-gh-dszmolka added question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage Issue is under initial triage status-information_needed Additional information is required from the reporter labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

2 participants