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

PoolCluster: access to pool events, lack of docs #2468

Open
singerb opened this issue Feb 25, 2021 · 2 comments
Open

PoolCluster: access to pool events, lack of docs #2468

singerb opened this issue Feb 25, 2021 · 2 comments

Comments

@singerb
Copy link

singerb commented Feb 25, 2021

I started experimenting with using PoolCluster to manage pools to multiple databases, including distinguishing readonly from readwrite (i.e. primary and replicas) via aliases and wildcards. I ran into trouble for a few reasons:

  • the example code given suggests that cluster.of() returns a Pool, when in fact it simply returns something that implements getConnection() and query(); after reading other issues, this of course makes sense because with a selector like READONLY* that naturally gives you an object that might represent N different pools
  • lack of explicit documentation here means I (and others) have had to infer this from testing and reading the source/issues
  • we need access to the Pool objects, however, to use things like the connection event (to set SQL session modes), and acquire/enqueue (for deadlock tracking); given the design constraint on of(), there needs to be another way to provide this access. Possibilities:
  • Allow passing full Pool objects to add(), not just PoolConfig; then the caller could do any setup needed before putting them in the cluster
  • Allow an optional callback to add() which will be called with the resulting Pool
  • Emit a pool or added event, similar to connection from Pool, that allows the listener to know when a new Pool has been created by the cluster and set listeners etc. on it
  • Allow passing event listeners in the config (would make sense as a larger design change, i.e. for all object types)

I know the documentation concerns have been raised before, but wanted to capture my thoughts on the other issues.

@dougwilson
Copy link
Member

Hi @singerb thank you for your thoughts. The entire PoolCluster system was a contribution by another user and we graciously accepted. I understand they did not document it very well, but if folks like yourself can help by making PRs improving these docs, that would be a huge help to others 👍 . I didn't fully read through all your concerns/comments above, but they seem to be various items like asking for new features, fixes, and docs. I would highly recommend splitting each separate ask into a separate issue, as otherwise it makes the issue seem like a lot for those who may want to contribute just part of it, and of course the more things in a single issue, the more comments it get, and the harder it is in the future to understand what has been done, what has not been done, what has changed, etc.

@iamlaobie
Copy link

I have the same questions when I upgrade my project to poolCluster from single pool.
I read source code and find connection initial method like this:

  const { charset, timezone } = options ;
  Object.keys(pool._nodes).forEach(key => {
    pool._nodes[key].pool.on('connection', (connection) => {
      if (timezone) connection.query(`SET SESSION time_zone = '${timezone}'`);
      if (charset) connection.query(`SET NAMES ${charset}`);
    });
  });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants