-
Notifications
You must be signed in to change notification settings - Fork 428
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
Make RDBMS query timeout configurable #3728
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
77bf0cb
to
a690251
Compare
Codecov Report
@@ Coverage Diff @@
## master #3728 +/- ##
==========================================
- Coverage 82.28% 82.24% -0.04%
==========================================
Files 526 526
Lines 33853 33853
==========================================
- Hits 27856 27843 -13
- Misses 5997 6010 +13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test case to config_parser_SUITE
.
src/config/mongoose_config_spec.erl
Outdated
@@ -555,6 +555,8 @@ outgoing_pool_connection(<<"rdbms">>) -> | |||
validate = {enum, [odbc, pgsql, mysql]}}, | |||
<<"keepalive_interval">> => #option{type = integer, | |||
validate = positive}, | |||
<<"query_timeout">> => #option{type = integer, | |||
validate = positive}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate = positive}, | |
validate = positive}, |
test/common/config_parser_helper.erl
Outdated
@@ -836,7 +836,7 @@ default_pool_conn_opts(redis) -> | |||
database => 0, | |||
password => ""}; | |||
default_pool_conn_opts(rdbms) -> | |||
#{max_start_interval => 30}; | |||
#{query_timeout => 5, max_start_interval => 30}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 5 if the default is 5000?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confusing seconds with milliseconds 🤦🏽
A strict hardcoded macro is too inflexible.
a690251
to
57d3ba6
Compare
small_tests_24 / small_tests / 57d3ba6 small_tests_25 / small_tests / 57d3ba6 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 57d3ba6 ldap_mnesia_24 / ldap_mnesia / 57d3ba6 dynamic_domains_mysql_redis_25 / mysql_redis / 57d3ba6 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 57d3ba6 ldap_mnesia_25 / ldap_mnesia / 57d3ba6 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 57d3ba6 pgsql_mnesia_24 / pgsql_mnesia / 57d3ba6 internal_mnesia_25 / internal_mnesia / 57d3ba6 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 57d3ba6 mysql_redis_25 / mysql_redis / 57d3ba6 pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,{{badmatch,[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2201@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2201@localhost/res1">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"unsubscribed">>}],
[]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,{{badmatch,[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2215@localhost/res1">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2215@localhost/res1">>},
{<<"type">>,<<"unavailable">>}],
[]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pubsub_SUITE:tree+node_config:send_last_published_item_test{error,{{badmatch,false},
[{pubsub_tools,check_response,2,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,491}]},
{pubsub_tools,receive_response,3,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,481}]},
{pubsub_tools,receive_subscribe_response,3,
[{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
{line,334}]},
{pubsub_SUITE,'-send_last_published_item_test/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pubsub_SUITE.erl"},
{line,937}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pgsql_mnesia_25 / pgsql_mnesia / 57d3ba6 mssql_mnesia_25 / odbc_mssql_mnesia / 57d3ba6 riak_mnesia_24 / riak_mnesia / 57d3ba6 mysql_redis_25 / mysql_redis / 57d3ba6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
A strict hardcoded macro is too inflexible and this is a parameter I want to iterate on the configuration easily.