Re: pg_autovacuum patch for 7.4.2 and HEAD

Tom Lane wrote:

>> "Matthew T. O'Connor" <matthew@zeut.net> writes:
>>
>>> !     new_tbl->relid = atol(PQgetvalue(res, row, PQfnumber(res,
>>> "oid")));
>>> !     new_tbl->reltuples = atof(PQgetvalue(res, row, PQfnumber(res,
>>> "reltuples")));
>>> !     new_tbl->relpages = atol(PQgetvalue(res, row, PQfnumber(res,
>>> "relpages")));
>>
>>
>> This ignores the fact that relid and relpages are unsigned.  I would
>> suggest adopting the same convention for OID as is used in pg_dump and
>> other places:
>>
>> #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
>>
>> You could actually use this same macro for reading relpages, but that's
>> probably abusing the notation.  I'd use strtoul directly for relpages,
>> I think.
>>
>>> ! init_dbinfo(char *dbname, int oid, int age)
>>> ...
>>> ! init_dbinfo(char *dbname, uint oid, uint age)
>>>
>>
>>
>> This (and other declarations) should be "Oid oid".
>
>
> Thanks for the help / review.  Here is my 2nd cut at fixing this.  I
> believe I have addressed the above concernes.  Please review again and
> (hopefully) apply.


Ack... sorry.  This time with the attachment....
*** ./pg_autovacuum.c.orig    2004-02-28 18:08:10.000000000 -0500
--- ./pg_autovacuum.c    2004-03-13 11:27:35.918998127 -0500
***************
*** 117,125 ****
           atol(PQgetvalue(res, row, PQfnumber(res, "n_tup_upd"))));
      new_tbl->curr_vacuum_count = new_tbl->CountAtLastVacuum;

!     new_tbl->relid = atoi(PQgetvalue(res, row, PQfnumber(res, "oid")));
!     new_tbl->reltuples = atoi(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
!     new_tbl->relpages = atoi(PQgetvalue(res, row, PQfnumber(res, "relpages")));

      if (strcmp("t", PQgetvalue(res, row, PQfnumber(res, "relisshared"))))
          new_tbl->relisshared = 0;
--- 117,125 ----
           atol(PQgetvalue(res, row, PQfnumber(res, "n_tup_upd"))));
      new_tbl->curr_vacuum_count = new_tbl->CountAtLastVacuum;

!     new_tbl->relid = atooid(PQgetvalue(res, row, PQfnumber(res, "oid")));
!     new_tbl->reltuples = atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
!     new_tbl->relpages = atooid(PQgetvalue(res, row, PQfnumber(res, "relpages")));

      if (strcmp("t", PQgetvalue(res, row, PQfnumber(res, "relisshared"))))
          new_tbl->relisshared = 0;
***************
*** 159,166 ****
          if (res != NULL)
          {
              tbl->reltuples =
!                 atoi(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
!             tbl->relpages = atoi(PQgetvalue(res, 0, PQfnumber(res, "relpages")));

              /*
               * update vacuum thresholds only of we just did a vacuum
--- 159,166 ----
          if (res != NULL)
          {
              tbl->reltuples =
!                 atof(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
!             tbl->relpages = atooid(PQgetvalue(res, 0, PQfnumber(res, "relpages")));

              /*
               * update vacuum thresholds only of we just did a vacuum
***************
*** 237,243 ****
              for (i = 0; i < t; i++)
              {                    /* loop through result set looking for a
                                   * match */
!                 if (tbl->relid == atoi(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
--- 237,243 ----
              for (i = 0; i < t; i++)
              {                    /* loop through result set looking for a
                                   * match */
!                 if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
***************
*** 267,273 ****
              while (tbl_elem != NULL)
              {
                  tbl = ((tbl_info *) DLE_VAL(tbl_elem));
!                 if (tbl->relid == atoi(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
--- 267,273 ----
              while (tbl_elem != NULL)
              {
                  tbl = ((tbl_info *) DLE_VAL(tbl_elem));
!                 if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
***************
*** 361,369 ****
  {
      sprintf(logbuffer, "  table name:     %s.%s", tbl->dbi->dbname, tbl->table_name);
      log_entry(logbuffer);
!     sprintf(logbuffer, "     relid: %i;   relisshared: %i", tbl->relid, tbl->relisshared);
      log_entry(logbuffer);
!     sprintf(logbuffer, "     reltuples: %i;  relpages: %i", tbl->reltuples, tbl->relpages);
      log_entry(logbuffer);
      sprintf(logbuffer, "     curr_analyze_count:  %li; cur_delete_count:   %li",
              tbl->curr_analyze_count, tbl->curr_vacuum_count);
--- 361,369 ----
  {
      sprintf(logbuffer, "  table name:     %s.%s", tbl->dbi->dbname, tbl->table_name);
      log_entry(logbuffer);
!     sprintf(logbuffer, "     relid: %u;   relisshared: %i", tbl->relid, tbl->relisshared);
      log_entry(logbuffer);
!     sprintf(logbuffer, "     reltuples: %f;  relpages: %u", tbl->reltuples, tbl->relpages);
      log_entry(logbuffer);
      sprintf(logbuffer, "     curr_analyze_count:  %li; cur_delete_count:   %li",
              tbl->curr_analyze_count, tbl->curr_vacuum_count);
***************
*** 407,414 ****
      if (dbs->conn != NULL)
      {
          res = send_query(FROZENOID_QUERY, dbs);
!         dbs->oid = atoi(PQgetvalue(res, 0, PQfnumber(res, "oid")));
!         dbs->age = atoi(PQgetvalue(res, 0, PQfnumber(res, "age")));
          if (res)
              PQclear(res);

--- 407,414 ----
      if (dbs->conn != NULL)
      {
          res = send_query(FROZENOID_QUERY, dbs);
!         dbs->oid = atooid(PQgetvalue(res, 0, PQfnumber(res, "oid")));
!         dbs->age = atol(PQgetvalue(res, 0, PQfnumber(res, "age")));
          if (res)
              PQclear(res);

***************
*** 421,427 ****
  /* Simple function to create an instance of the dbinfo struct
      Initalizes all the pointers and connects to the database  */
  db_info *
! init_dbinfo(char *dbname, int oid, int age)
  {
      db_info    *newdbinfo = (db_info *) malloc(sizeof(db_info));

--- 421,427 ----
  /* Simple function to create an instance of the dbinfo struct
      Initalizes all the pointers and connects to the database  */
  db_info *
! init_dbinfo(char *dbname, Oid oid, long age)
  {
      db_info    *newdbinfo = (db_info *) malloc(sizeof(db_info));

***************
*** 500,506 ****
              for (i = 0; i < t; i++)
              {                    /* loop through result set looking for a
                                   * match */
!                 if (dbi->oid == atoi(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;

--- 500,506 ----
              for (i = 0; i < t; i++)
              {                    /* loop through result set looking for a
                                   * match */
!                 if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;

***************
*** 508,514 ****
                       * update the dbi->age so that we ensure
                       * xid_wraparound won't happen
                       */
!                     dbi->age = atoi(PQgetvalue(res, i, PQfnumber(res, "age")));
                      break;
                  }
              }
--- 508,514 ----
                       * update the dbi->age so that we ensure
                       * xid_wraparound won't happen
                       */
!                     dbi->age = atol(PQgetvalue(res, i, PQfnumber(res, "age")));
                      break;
                  }
              }
***************
*** 536,542 ****
              while (db_elem != NULL)
              {
                  dbi = ((db_info *) DLE_VAL(db_elem));
!                 if (dbi->oid == atoi(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
--- 536,542 ----
              while (db_elem != NULL)
              {
                  dbi = ((db_info *) DLE_VAL(db_elem));
!                 if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
                  {
                      found_match = 1;
                      break;
***************
*** 548,555 ****
              {
                  DLAddTail(db_list, DLNewElem(init_dbinfo
                            (PQgetvalue(res, i, PQfnumber(res, "datname")),
!                          atoi(PQgetvalue(res, i, PQfnumber(res, "oid"))),
!                       atoi(PQgetvalue(res, i, PQfnumber(res, "age"))))));
                  if (args->debug >= 1)
                  {
                      sprintf(logbuffer, "added database: %s", ((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
--- 548,555 ----
              {
                  DLAddTail(db_list, DLNewElem(init_dbinfo
                            (PQgetvalue(res, i, PQfnumber(res, "datname")),
!                          atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))),
!                       atol(PQgetvalue(res, i, PQfnumber(res, "age"))))));
                  if (args->debug >= 1)
                  {
                      sprintf(logbuffer, "added database: %s", ((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
***************
*** 681,687 ****
      sprintf(logbuffer, "dbname: %s Username %s Passwd %s", dbi->dbname,
              dbi->username, dbi->password);
      log_entry(logbuffer);
!     sprintf(logbuffer, " oid %i InsertThresh: %i  DeleteThresh: %i", dbi->oid,
              dbi->analyze_threshold, dbi->vacuum_threshold);
      log_entry(logbuffer);
      if (dbi->conn != NULL)
--- 681,687 ----
      sprintf(logbuffer, "dbname: %s Username %s Passwd %s", dbi->dbname,
              dbi->username, dbi->password);
      log_entry(logbuffer);
!     sprintf(logbuffer, " oid %u InsertThresh: %li  DeleteThresh: %li", dbi->oid,
              dbi->analyze_threshold, dbi->vacuum_threshold);
      log_entry(logbuffer);
      if (dbi->conn != NULL)
***************
*** 1072,1078 ****
                          {        /* Loop through tables in list */
                              tbl = ((tbl_info *) DLE_VAL(tbl_elem));        /* set tbl_info =
                                                                           * current_table */
!                             if (tbl->relid == atoi(PQgetvalue(res, j, PQfnumber(res, "oid"))))
                              {
                                  tbl->curr_analyze_count =
                                      (atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
--- 1072,1078 ----
                          {        /* Loop through tables in list */
                              tbl = ((tbl_info *) DLE_VAL(tbl_elem));        /* set tbl_info =
                                                                           * current_table */
!                             if (tbl->relid == atooid(PQgetvalue(res, j, PQfnumber(res, "oid"))))
                              {
                                  tbl->curr_analyze_count =
                                      (atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
*** ./pg_autovacuum.h.orig    2004-02-28 18:08:16.000000000 -0500
--- ./pg_autovacuum.h    2004-03-13 11:29:08.612786893 -0500
***************
*** 37,46 ****
  #define TABLE_STATS_QUERY    "select
a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del
frompg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r'" 

  #define FRONTEND
! #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%i"
  #define FROZENOID_QUERY "select oid,age(datfrozenxid) from pg_database where datname = 'template1'"
  #define FROZENOID_QUERY2 "select oid,datname,age(datfrozenxid) from pg_database where datname!='template0'"

  /* define cmd_args stucture */
  struct cmdargs
  {
--- 37,49 ----
  #define TABLE_STATS_QUERY    "select
a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del
frompg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r'" 

  #define FRONTEND
! #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%u"
  #define FROZENOID_QUERY "select oid,age(datfrozenxid) from pg_database where datname = 'template1'"
  #define FROZENOID_QUERY2 "select oid,datname,age(datfrozenxid) from pg_database where datname!='template0'"

+ /* define atooid */
+ #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
+
  /* define cmd_args stucture */
  struct cmdargs
  {
***************
*** 67,75 ****
      I think we need to guarantee this happens approx every 1Million TX's  */
  struct dbinfo
  {
!     int            oid,
!                 age;
!     int            analyze_threshold,
                  vacuum_threshold;        /* Use these as defaults for table
                                           * thresholds */
      PGconn       *conn;
--- 70,78 ----
      I think we need to guarantee this happens approx every 1Million TX's  */
  struct dbinfo
  {
!     Oid            oid;
!     long        age;
!     long        analyze_threshold,
                  vacuum_threshold;        /* Use these as defaults for table
                                           * thresholds */
      PGconn       *conn;
***************
*** 84,92 ****
  {
      char       *schema_name,
                 *table_name;
!     int            relid,
!                 reltuples,
!                 relisshared,
                  relpages;
      long        analyze_threshold,
                  vacuum_threshold;
--- 87,95 ----
  {
      char       *schema_name,
                 *table_name;
!     float        reltuples;
!     int            relisshared;
!     Oid            relid,
                  relpages;
      long        analyze_threshold,
                  vacuum_threshold;
***************
*** 111,117 ****

  /* Functions for managing database lists */
  static Dllist *init_db_list(void);
! static db_info *init_dbinfo(char *dbname, int oid, int age);
  static void update_db_list(Dllist *db_list);
  static void remove_db_from_list(Dlelem *db_to_remove);
  static void print_db_info(db_info * dbi, int print_table_list);
--- 114,120 ----

  /* Functions for managing database lists */
  static Dllist *init_db_list(void);
! static db_info *init_dbinfo(char *dbname, Oid oid, long age);
  static void update_db_list(Dllist *db_list);
  static void remove_db_from_list(Dlelem *db_to_remove);
  static void print_db_info(db_info * dbi, int print_table_list);