Re: [PATCHES] autovacuum multiworkers, patch 5
Alvaro Herrera [EMAIL PROTECTED] wrote: I manually merged your patch on top of my own. This is the result. Please have a look at whether the new code is correct and behaves sanely (I haven't tested it). Huh, you are right, it is broken, even in my outgoing mailbox -- I don't know what happened, as the file I have on disk is complete. Here is attached again. I tested your patch on Linux and Windows. It works well on Linux, where we use fork(), but falls into segfault on Windows, where we use exec(). Maybe you forgot to initialize the shared memory stuff. (I haven't find out where to be fixed, sorry.) Multiworker and balancing seem to work well after they successfully start up. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] autovacuum multiworkers, patch 5
Alvaro Herrera wrote: ITAGAKI Takahiro wrote: I tested your patch on Linux and Windows. It works well on Linux, where we use fork(), but falls into segfault on Windows, where we use exec(). Maybe you forgot to initialize the shared memory stuff. (I haven't find out where to be fixed, sorry.) Ok, thanks, this confirms that I have to try the EXEC_BACKEND code path. Oh, uh, the problem is that CreateSharedMemoryAndSemaphores wants to have access to the PGPROC already, but to obtain the PGPROC we need access to autovac shared memory (per AutoVacuumGetFreeProc). So this wasn't too bright a choice :-( -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] autovacuum multiworkers, patch 5
ITAGAKI Takahiro wrote: Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. One thing I forgot to mention is that this is unlikely to be implemented in 8.3. This is a WIP cost balancing patch built on autovacuum-multiworkers-5.patch. The total cost of workers are adjusted to autovacuum_vacuum_cost_delay. I manually merged your patch on top of my own. This is the result. Please have a look at whether the new code is correct and behaves sanely (I haven't tested it). -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Index: src/backend/commands/vacuum.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/vacuum.c,v retrieving revision 1.349 diff -c -p -r1.349 vacuum.c *** src/backend/commands/vacuum.c 14 Mar 2007 18:48:55 - 1.349 --- src/backend/commands/vacuum.c 11 Apr 2007 23:43:23 - *** vacuum_delay_point(void) *** 3504,3509 --- 3504,3512 VacuumCostBalance = 0; + /* update balance values for workers */ + AutoVacuumUpdateDelay(); + /* Might have gotten an interrupt while sleeping */ CHECK_FOR_INTERRUPTS(); } Index: src/backend/postmaster/autovacuum.c === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/autovacuum.c,v retrieving revision 1.40 diff -c -p -r1.40 autovacuum.c *** src/backend/postmaster/autovacuum.c 28 Mar 2007 22:17:12 - 1.40 --- src/backend/postmaster/autovacuum.c 11 Apr 2007 23:43:31 - *** *** 43,48 --- 43,49 #include storage/proc.h #include storage/procarray.h #include storage/sinval.h + #include storage/spin.h #include tcop/tcopprot.h #include utils/flatfiles.h #include utils/fmgroids.h *** *** 52,57 --- 53,59 #include utils/syscache.h + static volatile sig_atomic_t got_SIGUSR1 = false; static volatile sig_atomic_t got_SIGHUP = false; static volatile sig_atomic_t avlauncher_shutdown_request = false; *** static volatile sig_atomic_t avlauncher_ *** 59,64 --- 61,67 * GUC parameters */ bool autovacuum_start_daemon = false; + int autovacuum_max_workers; int autovacuum_naptime; int autovacuum_vac_thresh; double autovacuum_vac_scale; *** int autovacuum_freeze_max_age; *** 69,75 int autovacuum_vac_cost_delay; int autovacuum_vac_cost_limit; ! /* Flag to tell if we are in the autovacuum daemon process */ static bool am_autovacuum_launcher = false; static bool am_autovacuum_worker = false; --- 72,78 int autovacuum_vac_cost_delay; int autovacuum_vac_cost_limit; ! /* Flags to tell if we are in an autovacuum process */ static bool am_autovacuum_launcher = false; static bool am_autovacuum_worker = false; *** static int default_freeze_min_age; *** 82,95 /* Memory context for long-lived data */ static MemoryContext AutovacMemCxt; ! /* struct to keep list of candidate databases for vacuum */ ! typedef struct autovac_dbase { ! Oid ad_datid; ! char *ad_name; ! TransactionId ad_frozenxid; ! PgStat_StatDBEntry *ad_entry; ! } autovac_dbase; /* struct to keep track of tables to vacuum and/or analyze, in 1st pass */ typedef struct av_relation --- 85,106 /* Memory context for long-lived data */ static MemoryContext AutovacMemCxt; ! /* struct to keep track of databases in launcher */ ! typedef struct avl_dbase { ! Oid adl_datid; /* hash key -- must be first */ ! TimestampTz adl_next_worker; ! int adl_score; ! } avl_dbase; ! ! /* struct to keep track of databases in worker */ ! typedef struct avw_dbase ! { ! Oid adw_datid; ! char *adw_name; ! TransactionId adw_frozenxid; ! PgStat_StatDBEntry *adw_entry; ! } avw_dbase; /* struct to keep track of tables to vacuum and/or analyze, in 1st pass */ typedef struct av_relation *** typedef struct autovac_table *** 110,123 int at_vacuum_cost_limit; } autovac_table; typedef struct { ! Oid process_db; /* OID of database to process */ ! int worker_pid; /* PID of the worker process, if any */ } AutoVacuumShmemStruct; static AutoVacuumShmemStruct *AutoVacuumShmem; #ifdef EXEC_BACKEND static pid_t avlauncher_forkexec(void); static pid_t avworker_forkexec(void); --- 121,195 int at_vacuum_cost_limit; } autovac_table; + /*- + * This struct holds information about a single worker's whereabouts. We keep + * an array of these in shared memory, sized according to + *
Re: [PATCHES] autovacuum multiworkers, patch 5
Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. One thing I forgot to mention is that this is unlikely to be implemented in 8.3. This is a WIP cost balancing patch built on autovacuum-multiworkers-5.patch. The total cost of workers are adjusted to autovacuum_vacuum_cost_delay. I added copy of worker's cost parameters to the shared WorkerInfo array. A launcher and each worker reads and writes the copied parameters when a worker starts a vacuum job or exit the process. Workers assign their local VacuumCostDelay from the shared value every sleep in vacuum_delay_point(). I agree that mb_per_second or io_per_second are easier to use than present cost delay parameters, but we need more research to move to it. I think it is better to keep cost_limit and cost_delay as of 8.3, but we need cost-balanced multiworkers at any rate. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center autovacuum_balance.patch Description: Binary data ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] autovacuum multiworkers, patch 5
ITAGAKI Takahiro wrote: Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. One thing I forgot to mention is that this is unlikely to be implemented in 8.3. This is a WIP cost balancing patch built on autovacuum-multiworkers-5.patch. The total cost of workers are adjusted to autovacuum_vacuum_cost_delay. I added copy of worker's cost parameters to the shared WorkerInfo array. A launcher and each worker reads and writes the copied parameters when a worker starts a vacuum job or exit the process. Workers assign their local VacuumCostDelay from the shared value every sleep in vacuum_delay_point(). Thanks! I had already incorporated the foreach_worker changes into my code, and later realized that there's an important bug regarding the PGPROC of the workers, so I've reworked the patch, which meant that the foreach_worker() macro went away completely. I'll put it your changes in my current WIP patch; if you do any further work on it, please let me have it to include it in the latest work. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] autovacuum multiworkers, patch 5
Another problem seems to be that I'm not checking anywhere that a regular connection (not autovac) is not using an autovac-reserved PGPROC slot :-( I think I should tweak the logic that deals with ReservedBackends but it doesn't look entirely trivial. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] autovacuum multiworkers, patch 5
Hi, Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. Modulo two low probability failure scenarios, I feel this patch is ready to be applied; I will do so on Friday unless there are objections. The failure scenarios are detailed in the comment pasted below. I intend to attack these problems next, but as the first one should be fairly low probability, I don't think it should bar the current patch from being applied. (The second problem, which seems to me to be the most serious, should be easily fixable by checking launch times and aborting processes that took longer than autovacuum_naptime to start). /* * Main loop for the autovacuum launcher process. * * The signalling between launcher and worker is as follows: * * When the worker has finished starting up, it stores its PID in wi_workerpid * and sends a SIGUSR1 signal to the launcher. The launcher then knows that * the postmaster is ready to start a new worker. We do it this way because * otherwise we risk calling SendPostmasterSignal() when the postmaster hasn't * yet processed the last one, in which case the second signal would be lost. * This is only useful when two workers need to be started close to one * another, which should be rare but it's possible. * * Additionally, when the worker is finished with the vacuum work, it sets the * wi_finished flag and sends a SIGUSR1 signal to the launcher. Upon receipt * of this signal, the launcher then clears the entry for future use and may * start another worker right away, if need be. * * There is at least one race condition here: if the workers are all busy, a * database needs immediate attention and a worker finishes just after the * launcher started a worker and sent the signal to postmaster, but before * postmaster processes the signal; at this point, the launcher receives a * signal from the finishing process, sees the empty slot, and sends the * signal to postmaster again to start another worker. But the postmaster * SendPostmasterSignal() flag was already set, so the signal is lost. To * avoid this problem, the launcher should not try to start a new worker until * all WorkerInfo entries that have the wi_dboid field set have a PID assigned. * FIXME someday. The problem is that if we have workers failing to start for * some reason, holding the start of new workers will worsen the starvation by * disabling the start of a new worker as soon as one worker fails to start. * So it's important to be able to distinguish a worker that has failed * starting from a worker that is just taking its little bit of time to do so. * * There is another potential problem if, for some reason, a worker starts and * is not able to finish correctly. It will not be able to set its finished * flag, so the launcher will believe that it's still starting up. To prevent * this problem, we should check the PGPROCs of worker processes, and clean * them up if we find they are not actually running (or they correspond to * processes that are not autovacuum workers.) FIXME someday. */ -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] autovacuum multiworkers, patch 5
Alvaro Herrera wrote: Hi, uhmmm patch? Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. Modulo two low probability failure scenarios, I feel this patch is ready to be applied; I will do so on Friday unless there are objections. The failure scenarios are detailed in the comment pasted below. I intend to attack these problems next, but as the first one should be fairly low probability, I don't think it should bar the current patch from being applied. (The second problem, which seems to me to be the most serious, should be easily fixable by checking launch times and -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] autovacuum multiworkers, patch 5
Alvaro Herrera [EMAIL PROTECTED] wrote: Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. I'm interested in your multiworkers autovacuum proposal. I'm researching the impact of multiworkers with autovacuum_vacuum_cost_limit. Autovacuum will consume server resources up to autovacuum_max_workers times as many as before. I think we might need to change the semantics of autovacuum_vacuum_cost_limit when we have multiworkers. BTW, I found an unwitting mistake in the foreach_worker() macro. These two operations are same in C. - worker + 1 - (WorkerInfo *) (((char *) worker) + sizeof(WorkerInfo)) #define foreach_worker(_i, _worker) \ _worker = (WorkerInfo *) (AutoVacuumShmem + \ offsetof(AutoVacuumShmemStruct, av_workers)); \ for (_i = 0; _i autovacuum_max_workers; _i++, _worker += sizeof(WorkerInfo)) should be: #define foreach_worker(_worker) \ for ((_worker) = AutoVacuumShmem-av_workers; \ (_worker) AutoVacuumShmem-av_workers + autovacuum_max_workers; \ (_worker)++) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] autovacuum multiworkers, patch 5
ITAGAKI Takahiro wrote: Alvaro Herrera [EMAIL PROTECTED] wrote: Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. I'm interested in your multiworkers autovacuum proposal. I'm researching the impact of multiworkers with autovacuum_vacuum_cost_limit. Autovacuum will consume server resources up to autovacuum_max_workers times as many as before. I think we might need to change the semantics of autovacuum_vacuum_cost_limit when we have multiworkers. Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. This way, the maximum use of IO resources by vacuum can be easily determined and limited by the DBA; certainly much simpler than the vacuum cost limiting feature. BTW, I found an unwitting mistake in the foreach_worker() macro. These two operations are same in C. - worker + 1 - (WorkerInfo *) (((char *) worker) + sizeof(WorkerInfo)) Ah, thanks. I had originally coded the macro like you suggest, but then during the development I needed to use the i variable as well, so I added it. Apparently later I removed that usage; I see that there are no such uses left in the current code. The + sizeof(WorkerInfo) part is just stupidity on my part, sorry about that. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] autovacuum multiworkers, patch 5
Alvaro Herrera wrote: ITAGAKI Takahiro wrote: Alvaro Herrera [EMAIL PROTECTED] wrote: Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. I'm interested in your multiworkers autovacuum proposal. I'm researching the impact of multiworkers with autovacuum_vacuum_cost_limit. Autovacuum will consume server resources up to autovacuum_max_workers times as many as before. I think we might need to change the semantics of autovacuum_vacuum_cost_limit when we have multiworkers. Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. This way, the maximum use of IO resources by vacuum can be easily determined and limited by the DBA; certainly much simpler than the vacuum cost limiting feature. +1 Joshua D. Drake -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] autovacuum multiworkers, patch 5
Joshua D. Drake wrote: Alvaro Herrera wrote: ITAGAKI Takahiro wrote: Alvaro Herrera [EMAIL PROTECTED] wrote: Here is the autovacuum patch I am currently working with. This is basically the same as the previous patch; I have tweaked the database list management so that after a change in databases (say a new database is created or a database is dropped), the list is recomputed to account for the change, keeping the ordering of the previous list. I'm interested in your multiworkers autovacuum proposal. I'm researching the impact of multiworkers with autovacuum_vacuum_cost_limit. Autovacuum will consume server resources up to autovacuum_max_workers times as many as before. I think we might need to change the semantics of autovacuum_vacuum_cost_limit when we have multiworkers. Yes, that's correct. Per previous discussion, what I actually wanted to do was to create a GUC setting to simplify the whole thing, something like autovacuum_max_mb_per_second or autovacuum_max_io_per_second. Then, have each worker use up to (max_per_second/active workers) as much IO resources. This way, the maximum use of IO resources by vacuum can be easily determined and limited by the DBA; certainly much simpler than the vacuum cost limiting feature. +1 One thing I forgot to mention is that this is unlikely to be implemented in 8.3. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly