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.

Sign up for GitHub

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

Jump to bottom

WIP - feedback requested - multipool support #101

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

WIP - feedback requested - multipool support #101

wants to merge 12 commits into from

Conversation

marcparadise
Copy link
Member

@marcparadise marcparadise commented Sep 2, 2016 •
edited
Loading

Looking for early feedback on these changes which Introduce multipool support to sqerl in a backward compatible way.

TODO:

  • make sure travis is passing
  • ensure both new and old configuration formats work.
  • determine if more tests are needed
  • revert local testing change
  • clean up history
  • clean up debug output

@marcparadise
simplify CT setup a little
f561b73 
Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
First, break allt he things
36525a4 
Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
And then make it work again
6df4992 
Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
fix transposed parameters to proplist_get. Add support to say none fo…
bee4547 
…r prepared statements

Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
use macro for the default pool name
52b8395 
Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
Correct test setup issues, and add a couple of mp tests.
0165398 
- pool2 was apparently not a valid name for registering the
  pool process under - while the pool was created, it did not get
  registerd and so checkouts against it failed.
- fixed broken statement ordering in pgsql_create.sql

Signed-off-by: Marc A. Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
revert temp local testing changs
1f3e7be 
Signed-off-by: Marc Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
support old-style config for single pools
23b3ef4 
Signed-off-by: Marc Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
add otp 18.1 to travis
914c4d2 
Signed-off-by: Marc Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
remove temp debug code
b20d620 
Signed-off-by: Marc Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
repoint envy to upstream
f4abff4 
Signed-off-by: Marc Paradise <[email protected]>
Signed-off-by: Marc A. Paradise <[email protected]>
@marcparadise
Copy link
Member Author

Some notes:

  • sqerl interface will continue to work as is. The main sqerl module now passes through most calls to sqerl_core, passing in a default pool named sqerl to ensure backwards compatibility.
  • sqerl_mp: this is the multipool interface. It's the same as the sqerl interface, but all exports require an additional initial argument of pool name (atom). It also passes through to sqerl_core.
  • A couple of specific multipool tests were added to verify that multipool is behaving when using a pool other than the default, but the basic multipool functionality is actually already covered when using the default interface - they hit the same paths.

seth
seth reviewed Sep 2, 2016
View reviewed changes
src/sqerl_client.erl
@@ -173,19 +162,25 @@ code_change(_OldVsn, State, _Extra) ->

%% @doc Call DB driver process
exec_driver({Call, QueryOrName, Args}, _From, #state{cb_mod=CBMod, cb_state=CBState, timeout=Timeout}=State) ->
?LOG_STATEMENT(QueryOrName, Args),
% Don't keep logging flag in state, so that it can be easily
Copy link
Contributor

Choose a reason for hiding this comment

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

Use %% for comments

@marcparadise marcparadise force-pushed the mp/multipool branch 3 times, most recently from ee80c1d to 12b7425 Compare November 21, 2016 20:28
@marcparadise
Integrate the pool interface into main API.
ec58312 
This extends the main `sqerl` API by adding `_with` versions
of public entry points.  These functions each take an initial parameter
of context, which can be created with `sqerl:make_context(PoolName)`
where PoolName must be configured as a valid alternate pool.

The context form of this (instead of an atom for pool) keeps future
options to extend the API to include more features beyond alternate
pools

Signed-off-by: Marc A. Paradise <[email protected]>
@tas50 tas50 removed the Signed CLA label Jan 3, 2019
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marcparadise @seth @tas50 @chef-supermarket