meteor / postgres-packages

Early preview of PostgreSQL support for Meteor (deprecated, here for historical reasons)
http://meteor-postgres.readthedocs.org/
158 stars 25 forks source link

Need a way to drop triggers and functions when app restarts/ends #34

Open rclai opened 9 years ago

rclai commented 9 years ago

Without an easy way to do this, triggers and functions will pile up on the database. I tried to do this, which was the same thing that Ben Green suggested here:

var closeAndExit = function() {
  // function that drops the triggers and functions
  // PG._livePg is the saved instance of the `PgLiveQuery` class that you guys created on app startup
  PG._livePg.stop();
  process.exit();
};

process.on('SIGTERM', closeAndExit);
process.on('SIGINT', closeAndExit);

But for some reason it doesn't work..

stubailo commented 9 years ago

Yeah this is definitely something we should be doing by default!

rclai commented 9 years ago

I noticed that the global channel name always changes, would it make sense to have a reserved channel name or is that not a good idea?

yoonghm commented 8 years ago

I feel that database structures shall be created only once, not everytime meteor server is started or created.

I believe meteor server listens to notification and update relevant rows to meteor clients. I propose this: During the startup of a meteor server a) Disable all triggers under its management b) Clear all its notification by listen until there is nothing left c) Enable all the above triggers

Slava commented 8 years ago

I mean, there is a good pattern for the stored procedures names like livequery_simple_pg_MjFR. Presumably, on the startup we could just clean up everything matching the regex livequery_simple_pg_.*.

stubailo commented 8 years ago

Sounds good to me. Anyone got a PR up their sleeve?

rclai commented 8 years ago

Maybe me. Should I tear down the triggers/function in this spot.

stubailo commented 8 years ago

That seems like it could work to me! @Slava should confirm, but he could probably look at the PR also.

marcmunro commented 8 years ago

Couldn't you just use names that don't keep changing? That way they will not pile up.

FWIW, in order to secure my database, I would like to see the meteor app connect to the database as a user that does not have the right to create or modify database objects. I would like to see an option to allow the triggers and functions to be managed outside of the meteor app - ie a do-not-mess-with-my-database option.

My current project is to integrate Meteor with a Postgres-based Virtual Private Database - so the whole database security thing is important to me. My app will be given only the most minimal database privileges.

Oh, and to establish my credentials I've been a database adminstrator for close to 20 years and have been using, and developing for, postgres since version 7.

yoonghm commented 8 years ago

I totally agree with @marcmunro on both issues a) Use the same name signature, such as same unique postfix. b) Do nothing to the triggers. I originally posted to ignore disabling/enabling of triggers. Later i looked the source code for postgres on pg_notify(), there is only one FIFO queue, it seems that once meteor server has listened to, even if it has stopped listening (killed or exited), notification messages will continue pile up. (Need someone to test this scenario) It may cause issues. However, as it is dead, meteor clients cannot interact with the database. It should be fine. DBA can still perform changes to the database, he or she should know what to do. If the messages are the same, only one is being stored. Also, it seems that the old one is removed. There is no function to clear the messag except actively listening to it.

So, the steps become a) Disable all triggers under its management b) Clear all its notification by by listening to it, until there is nothing left for itself, but not relaying to meteor clients c) Enable all the above triggers

rclai commented 8 years ago

In the meantime I have changed the channel name to a fixed name to prevent the pileup.

yoonghm commented 8 years ago

Here is my finding on postgresql notify/select

If the listening process exited, notification will NOT be queued.

So it is safe to ignore dropping and recreating the triggers. I looked up the code .../packages/pg/observe-driver/polling-driver.js, the _DROP TRIGGER ..._ code using the hash code for the _NEW_ trigger name. So, it can never delete the old trigger.

We can ignore this create/drop trigger staff. The creation of trigger should be done during migration process by knex.

marcmunro commented 8 years ago

Could I request that the channel name incorporate the database name? That way if there were multiple meteor apps on multiple databases, they would each have their own channel.

yoonghm commented 8 years ago

Yesterday I still cannot reason out why random hash code is needed for the trigger name. Can anyone point to me some architectural design document for PostgreSQL implementation within Meteor?

If more than one processes has registered their intends to the same channel (listen <channel name>), the notification messages will _NOT_ be deleted from the disk-based queue if one of the progress is still active but has not listened the message. We can use fixed channel name.

marcmunro commented 8 years ago

I hacked my own copy of the code to use a static channel name derived from the database name. It worked for me, but the means I used to get the database name seemed a bit hacky, so I'm not going to release the code. Is anyone going to fix this properly?

yoonghm commented 8 years ago

could i get from you? i have to manually delete the triggers one by one?

rclai commented 8 years ago

If you use PgAdmin 3, you can delete them really quickly.

marcmunro commented 8 years ago

@yoonghm - sorry I didn't see your email until now. Here is the code I used (applied at around line 4 of packages/pg/pg.js:

// MM: Hack to get the database name from the URL.  This is not the
// right way to do it - the app should be able to specify what database
// to connect to, but it seems the library does this before the app even
// starts.
PG.dbname = PG.defaultConnectionUrl.replace(/.*\//, "")
PG._npmModule = pg;

var livePg = new PgLiveQuery({
  connectionUrl: PG.defaultConnectionUrl,
  channel: 'simple_pg_' + PG.dbname
});
PG._livePg = livePg;

Sorry, I don't have a repo you can pull this from right now. If you have any trouble with it, please ask for help.

As for dropping the existing triggers, yes you have to delete them one by one, but try running this from psql:

select '': select 'drop function "' || proname || '"() cascade;' from pg_proc where proname like 'livequ%';

You can then simply paste the results in to the psql command line.