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

Fix strategy fifo as default if not defined. #6

Merged
2 commits merged into from
Mar 3, 2015

Conversation

RaoH
Copy link
Member

@RaoH RaoH commented Feb 24, 2015

Added fifo as default instead of lifo if it's not declared in pool args.
This is so each connection will be used so we minimize connection timeout from mysql.

Added fifo as default instead of lifo if it's not declared in pool args.
This is so each connection will be used so we minimize connection timeout from mysql.
%% We want strategy fifo as default instead of lifo.
PoolArgs1 = case proplists:is_defined(strategy, PoolArgs) of
true -> PoolArgs;
false -> PoolArgs ++ [{strategy, fifo}]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hellre [{strategy, fifo} | PoolArgs]. Inte för prestandan (för PoolArgs är ändå inte extremt lång) men det är mer normalt att lägga till saker först i en lista.

I kommentaren, skriv gärna varför vi vill ha fifo, t.ex. så här:

%% Use fifo by default. MySQL closes unused connections after a certain time.
%% Fifo causes all connections to be regularily used which prevents them from
%% being closed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Förresten, detta borde ligga i child_spec/3 istället för här.

@ghost
Copy link

ghost commented Feb 27, 2015

👍

ghost pushed a commit that referenced this pull request Mar 3, 2015
…n-not-defined

Fix strategy fifo as default if not defined.
@ghost ghost merged commit 4e0b842 into master Mar 3, 2015
@getong getong deleted the add-strategy-fifo-as-default-when-not-defined branch November 11, 2018 14:40
This pull request was closed.
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