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

Query timeout #394

Closed
budarin opened this issue Jun 6, 2022 · 18 comments
Closed

Query timeout #394

budarin opened this issue Jun 6, 2022 · 18 comments

Comments

@budarin
Copy link

budarin commented Jun 6, 2022

If shutdown Postgres server and make a query:

req.sql`select ...`;

it runs forever

Is there a parameter that would be responsible for the maximum query execution time in such a situation, after which an error would be generated?

@budarin
Copy link
Author

budarin commented Jun 8, 2022

maybe I'm asking banal things, but I have an alibi - I'm a beginner :)

well, is there a complete list of "Other connection parameters"?

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

Hi @budarin . No not at all :) You can see all parameters here : https://github.com/porsager/postgres#all-postgres-options

I don't see why that wouldn't throw though, only thing I can think of is it hanging in a half open tcp socket, but we're setting tcp keep alive by default at 60s, so that should be the default (unless you're using deno)

@budarin
Copy link
Author

budarin commented Jun 9, 2022

  connection : {
     application_name   : 'postgres.js', // Default application_name
     ...                                 // Other connection parameters
  },

I'm talking about this ^ parameters

we use Pg in docker and client on host
Have tried on Windows and Mac - the same behaviour on both platforms

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

These parameters are just passed directly on to the DB, and could be anything. Not really related to Postgres.js

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

Which version are you running? Are you sure that you don't receive an error if you wait at least 60s ?

@budarin
Copy link
Author

budarin commented Jun 9, 2022

I'm using "postgres": "^3.2.4",

I am sure that the service does not generate an error after 60 seconds - I waited for more than 3 minutes
It is strange that a client in the form of a docker service generates an error :

  Error: getaddrinfo ENOTFOUND pg
    at __node_internal_captureLargerStackTrace (node:internal/errors:466:5)
    at __node_internal_ (node:internal/errors:688:10)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:83:26)
   originCache.set(xs, new Error())
  Error
    at cachedError (webpack://raketa-service-boilerplate/node_modules/postgres/src/query.js:166:23)
    at constructor (webpack://raketa-service-boilerplate/node_modules/postgres/src/query.js:36:24)
    at apply (webpack://raketa-service-boilerplate/node_modules/postgres/src/index.js:105:11)
    at apply (webpack://raketa-service-boilerplate/src/server/plugins/pg/index.ts:37:36)",

and leads to a service crash but service runing in VS Code does not get the error

Is this normal behavior - the service crashes if it can't resolve the Pg name to the address?

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

It'll crash if you don't catch the error (depending on node version).

What node versions are you running (do they vary between vscode and docker)?

@budarin
Copy link
Author

budarin commented Jun 9, 2022

I'm using the same node v18.3.0 both there and there

@budarin
Copy link
Author

budarin commented Jun 9, 2022

By the way, what is the best typical code with error handling when executing requests for postgres.js ?

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

Do you think you can make a reproducible case for me?

Wrt. Error handling the idea is that you just handle your queries errors. So either try catch with await or call eg .catch(). Whatever fits your setup best.

@budarin
Copy link
Author

budarin commented Jun 9, 2022

Here is the code in health /status route

            return Promise.all([sql`select true as pong`, redis.ping()])
                .then(() => 'Ok')
                .catch((err) => {
                    void res.code(HTTP_SERVICE_UNAVAILABLE_CODE).header('Retry-After', 5);

                    req.log.error({
                        msg: 'Service is unavailable',
                        error: err as Error,
                        where: STATUS_ROUTE,
                        stack: (err as Error).stack,
                    });

                    return 'Error';
                });

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

Ok, and that never settles if you request it after doing a DB shutdown?

@budarin
Copy link
Author

budarin commented Jun 9, 2022

Yes.
There is no the error in log output (if postgres is running in docker and a service is running in VS Code)

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

I see. I think the keep alive might not be respected through docker then?

@budarin
Copy link
Author

budarin commented Jun 9, 2022

I suspect that there may be problems with proxying connections to the virtual machine on which docker is running - it is clear that in the docker environment, the postgresql client crashes immediately

by the way, this code has a catch error handler, but the service crashes

@porsager
Copy link
Owner

porsager commented Jun 9, 2022

Could that crash be from another query that you're not catching?

@budarin
Copy link
Author

budarin commented Jun 9, 2022

no - I wait for health check only without executing any queries

The worst thing is that the service crashes hard and the docker does not try to raise a new instance
When I use node-postgres in another service - this did not happen:(

@budarin
Copy link
Author

budarin commented Jun 11, 2022

I thought of another bad scenario - when there is another connection to subscribe to notify - it will crash as soon as a reconnection attempt is made when the postgres service crashes or not ready in docker/cuber - this often happens at the start of the cluster because the services are started in random order

it's critical for services in docker/cuber infrastructure - the service should not crash due to the fact that it temporarily cannot resolve the name to the ip address until the service has started or is temporarily unavailable

@budarin budarin closed this as completed Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants