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

Fixup types and metrics #1144

Open
wants to merge 3 commits into
base: kjs/pglite-2
Choose a base branch
from

Conversation

3commascapital
Copy link

No description provided.

@@ -57,6 +57,10 @@ export class MetricsService {
ponder_rpc_request_duration: prometheus.Histogram<"network" | "method">;
ponder_rpc_request_lag: prometheus.Histogram<"network" | "method">;

ponder_pglite_pool_connections: prometheus.Gauge<"pool" | "kind"> = null!;
Copy link
Collaborator

@0xOlias 0xOlias Oct 8, 2024

Choose a reason for hiding this comment

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

I'm pretty sure the PGlite driver does not maintain a connection pool in the traditional sense. It may have some kind of internal queue (which would be analogous to the node-postgres connection pool query queue) but it doesn't look like it's exposed.

For now, let's remove all pglite-specific metrics. We can still increment the ponder_postgres_query_total in the PGlite codepath, but the pool_connections and query_queue_size metrics will just remain at zero on the pglite path.

Comment on lines 151 to 160
const poolConfig: PoolConfig = {
max: 20,
};

// driver = {
// user: createSqliteDatabase(userFile),
// readonly: createReadonlySqliteDatabase(userFile),
// sync: createSqliteDatabase(syncFile),
// };
driver = {
internal: createPool(poolConfig),
user: createPool(poolConfig),
readonly: createReadonlyPool(poolConfig),
sync: createPool(poolConfig),
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

PGlite is single user/connection. And at the moment, it doesn't seem to support opening a database in readonly mode while another database is writing. So for now, we'll need to combine the internal, readonly and user drivers into one that we'll call user (those all touch the same tables). The sync can still be its own database and driver.

Here's what I'm thinking:

import { PGlite } from "pglite";

// ...

const userDir = path.join(args.databaseConfig.directory, "public");
const syncDir = path.join(args.databaseConfig.directory, "sync");

const userPglite = await PGlite.create(userDir)
const syncPglite = await PGlite.create(syncDir)

driver = {
  user: userPglite,
  sync: syncPglite,
}

The driver object should contain the low-level drivers so that we can safely tear them down them in the kill function.


qb = {
internal: new HeadlessKysely({
name: "internal",
common: args.common,
dialect: new SqliteDialect({ database: driver.user }),
dialect: new PostgresDialect({ pool: driver.internal }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now, when we create the query builder (Kysely) objects, we can use the kysely-pglite driver (https://github.com/dnlsandiego/kysely-pglite/tree/main).

We should use the user driver for the internal, user, and readonly qb objects. I think this qb object should look identical to the one below for postgres except for the dialect option.

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