pgdump/parallel.c: "aborting" flag is dead code

Поиск
Список
Период
Сортировка
От Tom Lane
Тема pgdump/parallel.c: "aborting" flag is dead code
Дата
Msg-id 15583.1464462418@sss.pgh.pa.us
обсуждение исходный текст
Список pgsql-hackers
parallel.c has an "aborting" flag that's entirely useless, as it's
set only during archive_close_connection(), and tested only in places
that are not reached after that.  I've confirmed this both by code
reading and by testing.  It appears from some of the comments that
there was once an intent to shut down workers by sending an explicit
TERMINATE message, rather than just closing the pipe, and in that
case maybe there would have been some value in this flag.  As is,
though, it just sows confusion, so I propose removing it as attached.

            regards, tom lane

diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index 56a74cb..e9e8698 100644
*** a/src/bin/pg_dump/parallel.c
--- b/src/bin/pg_dump/parallel.c
*************** static int    piperead(int s, char *buf, in
*** 95,105 ****

  #else                            /* !WIN32 */

! /*
!  * Variables for handling signals.  aborting is only ever used in the master,
!  * the workers just need wantAbort.
!  */
! static bool aborting = false;
  static volatile sig_atomic_t wantAbort = 0;

  /* Non-Windows implementation of pipe access */
--- 95,101 ----

  #else                            /* !WIN32 */

! /* Signal handler flag */
  static volatile sig_atomic_t wantAbort = 0;

  /* Non-Windows implementation of pipe access */
*************** archive_close_connection(int code, void
*** 301,314 ****
              if (si->AHX)
                  DisconnectDatabase(si->AHX);

- #ifndef WIN32
-
-             /*
-              * Setting aborting to true shuts off error/warning messages that
-              * are no longer useful once we start killing workers.
-              */
-             aborting = true;
- #endif
              ShutdownWorkersHard(si->pstate);
          }
          else
--- 297,302 ----
*************** select_loop(int maxFd, fd_set *workerset
*** 1178,1188 ****
          /*
           * If we Ctrl-C the master process, it's likely that we interrupt
           * select() here. The signal handler will set wantAbort == true and
!          * the shutdown journey starts from here. Note that we'll come back
!          * here later when we tell all workers to terminate and read their
!          * responses. But then we have aborting set to true.
           */
!         if (wantAbort && !aborting)
              exit_horribly(modulename, "terminated by user\n");

          if (i < 0 && errno == EINTR)
--- 1166,1174 ----
          /*
           * If we Ctrl-C the master process, it's likely that we interrupt
           * select() here. The signal handler will set wantAbort == true and
!          * the shutdown journey starts from here.
           */
!         if (wantAbort)
              exit_horribly(modulename, "terminated by user\n");

          if (i < 0 && errno == EINTR)
*************** sendMessageToWorker(ParallelState *pstat
*** 1279,1295 ****

      if (pipewrite(pstate->parallelSlot[worker].pipeWrite, str, len) != len)
      {
!         /*
!          * If we're already aborting anyway, don't care if we succeed or not.
!          * The child might have gone already.  (XXX but if we're aborting
!          * already, why are we here at all?)
!          */
! #ifndef WIN32
!         if (!aborting)
! #endif
!             exit_horribly(modulename,
!                         "could not write to the communication channel: %s\n",
!                           strerror(errno));
      }
  }

--- 1265,1273 ----

      if (pipewrite(pstate->parallelSlot[worker].pipeWrite, str, len) != len)
      {
!         exit_horribly(modulename,
!                       "could not write to the communication channel: %s\n",
!                       strerror(errno));
      }
  }


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Joe Conway
Дата:
Сообщение: Re: Does people favor to have matrix data type?
Следующее
От: Tom Lane
Дата:
Сообщение: Misdesigned command/status APIs for parallel dump/restore