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

Tratar erro de desconexão com banco de dados no descongelamento das lambdas #1570

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

aprendendofelipe
Copy link
Collaborator

As lambdas podem ser congeladas logo após responderem a requisição, mesmo se ainda existir código a ser executado depois do envio da resposta.

Isso não vinha sendo um problema, mas com a atualização do runtime para a v20 no PR #1565, começamos a receber erros ECONNRESET.

Especulo que otimizações do Node estão permitindo congelar a lambda mais rapidamente, enquanto ainda não foi executada a getConnectionLimits() ou a getOpenedConnections().

Se as lambdas são descongeladas no meio da execução de uma dessas funções (especificamente no meio da query), onde o banco de dados já vai ter fechado a conexão por timeout, é muito provável que seja o momento em que os erros ECONNRESET estão sendo produzidos.

Aproveitei para otimizar o processo de liberação da conexão, onde agora o pool não é mais destruído, mas apenas a conexão com o banco de dados é liberada através do argumento true passado para client.release() quando for estivermos com muitas conexões abertas (tooManyConnections).

Copy link

vercel bot commented Dec 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tabnews ✅ Ready (Inspect) Visit Preview Dec 13, 2023 4:47pm

@aprendendofelipe aprendendofelipe merged commit 9e4ae2e into main Dec 13, 2023
7 checks passed
@aprendendofelipe aprendendofelipe deleted the econnreset branch December 13, 2023 16:52
@filipedeschamps
Copy link
Owner

Especulo que otimizações do Node estão permitindo congelar a lambda mais rapidamente, enquanto ainda não foi executada a getConnectionLimits() ou a getOpenedConnections().

Se as lambdas são descongeladas no meio da execução de uma dessas funções (especificamente no meio da query), onde o banco de dados já vai ter fechado a conexão por timeout, é muito provável que seja o momento em que os erros ECONNRESET estão sendo produzidos.

Eu não sei se entendi muito bem, mas uma lambda só congela a execução depois de um response (e ela faz isso quase que de forma imediata). Enquanto não deu response, tudo que precisa ser executado vai ser executado.

@aprendendofelipe
Copy link
Collaborator Author

uma lambda só congela a execução depois de um response (e ela faz isso quase que de forma imediata). Enquanto não deu response, tudo que precisa ser executado vai ser executado.

Isso mesmo! E o problema deve ser por códigos que podem ficar para serem executados após o response, como é o caso do finally no trecho abaixo:

try {
client = options.transaction ? options.transaction : await tryToGetNewClientFromPool();
return await client.query(query);
} catch (error) {
throw parseQueryErrorAndLog(error, query);
} finally {
if (client && !options.transaction) {
const tooManyConnections = await checkForTooManyConnections(client);
client.release(tooManyConnections && webserver.isServerlessRuntime);
}

Por ser um erro esporádico, e por serem abertas muito menos lambdas em homologação do que no ambiente de produção, já está rodando em produção. 😅

@filipedeschamps
Copy link
Owner

Puuutts sensacionaaaaal 😍

@aprendendofelipe
Copy link
Collaborator Author

uma lambda só congela a execução depois de um response (e ela faz isso quase que de forma imediata).

Minha especulação é que esse "imediato" está mais "imediato" com a atualização, e por isso o erro só agora. Isso porque acredito que esse erro já poderia estar ocorrendo esporadicamente, mas tivemos sorte. 🍀

Logo saberemos se é isso, pois só foram tratados os erros ECONNRESET dentro desse finally. Se o erro estiver disparando em outro local, ainda teremos os alertas. Assim como saberemos se o tratamento desse erro fizer surgir outros erros, talvez na hora de executar o client.release(). 🤝

@aprendendofelipe
Copy link
Collaborator Author

Logo saberemos se é isso

Vou continuar investigando, pois já disparou um erro. 😒 E não havia disparado nenhuma vez entre o rollback da v16 e o deploy desse PR, então é algo com a nova versão do Node.

Sobre as mudanças desse PR, acredito que são válidas. Talvez futuramente encontremos outros erros (se não todos) que possam ser ignorados em:

if (error.code === 'ECONNRESET') {

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

Successfully merging this pull request may close these issues.

2 participants