On Mon, Dec 5, 2011 at 9:15 PM, Takatoshi MATSUO <[email protected]>wrote:
> Hello Serge > > Serge Dubrouski <[email protected]>: > > Hello - > > > > Takatoshi MATSUO did a tremendous job on implementing support for > streaming > > replication feature in pgsql RA. Also it looks like PostgeSQL 9.1 has all > > necessary interfaces to successfully implement Pacemaker's M/S concept. > So > > I think it's time to start discussion on how to merge Takatoshi's work > into > > pgsql RA baseline. Here is the link to Takatoshi's GitHUB if somebody > wants > > to test his RA: > > > > https://github.com/t-matsuo/ > > > > So far I tested it for backward compatibility in a standard > non-replication > > mode and also tested M/S model and found no real issues. Though it > > definitely requires some more polishing and testing. > > > > Takatoshi, here are some changes that I want to discuss with you: > > > > 1. Is it possible to add a check for PostgreSQL version and fail with > > OCF_ERR_INSTALLED when one tries to start replication on version less > than > > 9.1? A simple cat on PG_VERSION with some analysis would probably do. > > I'll add a check. > > > 2. I think that following lines should be moved from pgsql_start to > > pgsql_validate_all > > > > 535 # Check whether tmpdir is readable by pgdba user > > 536 if ! runasowner "test -r $OCF_RESKEY_tmpdir"; then > > 537 ocf_log err "Directory $OCF_RESKEY_tmpdir is not readable by > > $OCF_RESKEY_pgdba" > > 538 return $OCF_ERR_PERM > > 539 fi > > Thanks. I think so too. > I'll fix it. > > On a second thought I'm not sure why we need that parameter and directory at all. Why not to create rep_mode.conf, PGSQL.lock and xlog.note in $OCF_RESKEY_pgdata ? What problems it can create? One more advantage to do it in $OCF_RESKEY_pgdata is an ability to handle more than PostgreSQL instance on the same server without a need for additional temp directories. Also I think it would be good if RA was able to take care of adding "include $WHATEVER_DIR/rep_mode.conf" in postgresql.conf. It will make the RA self sustainable. In a current situation admin has add that directive manually. RA though can something like this in a start function for replication mode: if ! grep -i "include $WHATEVER_DIR/rep_mode.conf" $OCF_RESKEY_config then echo "include $WHATEVER_DIR/rep_mode.conf" >> $OCF_RESKEY_config fi Don't know if it makes sense to remove it on stop. > 3. I don't really like this part of pgsql_real_monitor: > > > > 775 if ! is_replication; then > > 776 OCF_RESKEY_monitor_sql=`escape_string > > "$OCF_RESKEY_monitor_sql"` > > 777 runasowner -q $loglevel "$OCF_RESKEY_psql $psql_options -c > > '$OCF_RESKEY_monitor_sql'" > > 778 rc=$? > > 779 else > > 780 output=`su $OCF_RESKEY_pgdba -c "cd $OCF_RESKEY_pgdata; > > $OCF_RESKEY_psql $psql_options -Atc \"${CHECK_MS_SQL}\""` > > 781 rc=$? > > 782 fi > > > > I think that functional monitor (the one that uses monitor_sql) should > run > > always independently of DB mode since its primary role is to check data > and > > fail if it's not correct or corrupted. In replication mode there should > be > > additional monitoring. Other way it misleads customer on a usage of > > monitor_sql. > > All right. > > Does it need to execute "select now();" if monitor_sql parameter is empty? > I think it's unnecessary. > For a case of an empty parameter you are right and running "select now()" probably unnecessary, Non-empty monitro_sql shall be executed in my opinion. > > > 4. You already populate several attributes with crm_attribute. Does it > make > > sense to populate a name of a Master node in promote function and use it > > later on instead of running crm_mon on each monitor command? > > > > Do you mean that you don't want to use crm_mon in monitor? > I prefer to have as few dependencies on external programs as possible. Using crm_attribute to communicate the name of a master node would be consistent with the rest of the script since you already use it for communicating state of the nodes. If you prefer using crm_mon than you have to add a check that that binary exists on the server. In 99.99% of the case it will but still the check is necessary I think. > > > 5. It also requires some changes in terms of a proper English but we can > go > > through it later. > > I hope your help. > Will do. > > > > > And yet again thanks for a brilliant work. > > > Thank you for the compliment. > > > Florian, Dejan how would you like to merge a patch when we are ready? The > > patch will be rather big one and AFAIK you have some policy on the > amount of > > changes for one patch. > > > > > > -- > > Serge Dubrouski. > > -- > Regards, > Takatoshi MATSUO > _______________________________________________________ > Linux-HA-Dev: [email protected] > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev > Home Page: http://linux-ha.org/ > -- Serge Dubrouski.
_______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
