Skip to content

cluster/stmt: Split registered statements by caller package#65

Merged
stgraber merged 1 commit into
canonical:mainfrom
masnax:multi-stmt
May 25, 2023
Merged

cluster/stmt: Split registered statements by caller package#65
stgraber merged 1 commit into
canonical:mainfrom
masnax:multi-stmt

Conversation

@masnax
Copy link
Copy Markdown
Contributor

@masnax masnax commented May 25, 2023

If a microcluster project indirectly imports microcluster from another project (i.e. microcloud using microceph, which both use microcluster), then the map of registered statements may end up polluted with entries from that project.

To avoid this, a call to microcluster.Start will record the name of the caller package (whether from snap build, GOPATH, or go module), and record it in the daemon.

As well, on start, when calls to RegisterStmt are executed, those packages will be recorded, and statements will be registered by their package.

Then, when preparing statements, only statements belonging to the daemon package, or to 'microcluster' (as that is ubiquitous) will be prepared.

If a microcluster project indirectly imports microcluster from another
project (i.e. microcloud using microceph, which both use microcluster),
then the map of registered statements may end up polluted with entries
from that project.

To avoid this, a call to microcluster.Start will record the name of the
caller package (whether from snap build, GOPATH, or go module), and
record it in the daemon.

As well, on start, when calls to `RegisterStmt` are executed, those
packages will be recorded, and statements will be registered by their
package.

Then, when preparing statements, only statements belonging to the daemon
package, or to 'microcluster' (as that is ubiquitous) will be prepared.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@masnax masnax requested a review from stgraber May 25, 2023 01:25
@stgraber
Copy link
Copy Markdown
Contributor

I'm confused as to why we need to specify the project in NewDaemon.

Given we have the logic to extract the project from the file path at the time of adding the statements, we should be able to similarly extract the caller's project at the time of preparing them, no?

@masnax
Copy link
Copy Markdown
Contributor Author

masnax commented May 25, 2023

I'm confused as to why we need to specify the project in NewDaemon.

Given we have the logic to extract the project from the file path at the time of adding the statements, we should be able to similarly extract the caller's project at the time of preparing them, no?

The package found by PrepareStmts will always be the go module path for microcluster because it's called internally in microcluster. The earliest point where we call into microcluster from microcloud is in microcluster.Start.

It's also done in an API call, so we can't go backward a constant amount and expect to find microcloud.

@stgraber
Copy link
Copy Markdown
Contributor

Ah, right, that makes sense.

@stgraber stgraber merged commit 9b4786b into canonical:main May 25, 2023
@roosterfish roosterfish mentioned this pull request Mar 18, 2025
roosterfish added a commit that referenced this pull request Mar 26, 2025
This is to explore the idea of not having the extra `project` field but
instead tracking both internal and external SQL statements together.

Ultimately this allows getting rid of the "get caller" concept.
It was introduced as part of
#65 but doesn't seem to be
required anymore.

Fixes #350.
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