pgbench: doExecuteCommand -> executeMetaCommand
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 27 Mar 2019 15:17:19 +0000 (12:17 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 27 Mar 2019 15:21:02 +0000 (12:21 -0300)
The new function is only in charge of meta commands, not SQL commands.
This change makes the code a little clearer: now all the state changes
are effected by advanceConnectionState.  It also removes one indent
level, which makes the diff look bulkier than it really is.

Author: Fabien Coelho
Reviewed-by: Kirk Jamison
Discussion: https://postgr.es/m/alpine.DEB.2.21.1811240904500.12627@lancre

src/bin/pgbench/pgbench.c

index cdd21d7469f65e1c7d9b06c6632bb9f85155adde..ac1322980871f4236dd65191261bf7e56139108d 100644 (file)
@@ -607,8 +607,8 @@ static void setIntValue(PgBenchValue *pv, int64 ival);
 static void setDoubleValue(PgBenchValue *pv, double dval);
 static bool evaluateExpr(TState *thread, CState *st, PgBenchExpr *expr,
             PgBenchValue *retval);
-static instr_time doExecuteCommand(TState *thread, CState *st,
-                instr_time now);
+static ConnectionStateEnum executeMetaCommand(TState *thread, CState *st,
+                  instr_time *now);
 static void doLog(TState *thread, CState *st,
      StatsData *agg, bool skipped, double latency, double lag);
 static void processXactStats(TState *thread, CState *st, instr_time *now,
@@ -802,7 +802,7 @@ out_of_range:
 invalid_syntax:
    if (!errorOK)
        fprintf(stderr,
-               "invalid input syntax for type bigint: \"%s\"\n",str);
+               "invalid input syntax for type bigint: \"%s\"\n", str);
    return false;
 }
 
@@ -827,7 +827,7 @@ strtodouble(const char *str, bool errorOK, double *dv)
    {
        if (!errorOK)
            fprintf(stderr,
-                   "invalid input syntax for type double: \"%s\"\n",str);
+                   "invalid input syntax for type double: \"%s\"\n", str);
        return false;
    }
    return true;
@@ -3012,6 +3012,8 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
     */
    for (;;)
    {
+       Command    *command;
+
        switch (st->state)
        {
                /* Select transaction (script) to run.  */
@@ -3151,8 +3153,10 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
                 * Send a command to server (or execute a meta-command)
                 */
            case CSTATE_START_COMMAND:
+               command = sql_script[st->use_file].commands[st->command];
+
                /* Transition to script end processing if done */
-               if (sql_script[st->use_file].commands[st->command] == NULL)
+               if (command == NULL)
                {
                    st->state = CSTATE_END_TX;
                    break;
@@ -3164,7 +3168,28 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
                    INSTR_TIME_SET_CURRENT_LAZY(now);
                    st->stmt_begin = now;
                }
-               now = doExecuteCommand(thread, st, now);
+
+               /* Execute the command */
+               if (command->type == SQL_COMMAND)
+               {
+                   if (!sendCommand(st, command))
+                   {
+                       commandFailed(st, "SQL", "SQL command send failed");
+                       st->state = CSTATE_ABORTED;
+                   }
+                   else
+                       st->state = CSTATE_WAIT_RESULT;
+               }
+               else if (command->type == META_COMMAND)
+               {
+                   /*-----
+                    * Possible state changes when executing meta commands:
+                    * - on errors CSTATE_ABORTED
+                    * - on sleep CSTATE_SLEEP
+                    * - else CSTATE_END_COMMAND
+                    */
+                   st->state = executeMetaCommand(thread, st, &now);
+               }
 
                /*
                 * We're now waiting for an SQL command to complete, or
@@ -3237,7 +3262,8 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
                            case IFSTATE_IGNORED:
                            case IFSTATE_ELSE_FALSE:
                                if (command->meta == META_IF)
-                                   conditional_stack_push(st->cstack, IFSTATE_IGNORED);
+                                   conditional_stack_push(st->cstack,
+                                                          IFSTATE_IGNORED);
                                else if (command->meta == META_ENDIF)
                                {
                                    Assert(!conditional_stack_empty(st->cstack));
@@ -3387,179 +3413,155 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 }
 
 /*
- * Subroutine for advanceConnectionState -- execute or initiate the current
- * command, and transition to next state appropriately.
+ * Subroutine for advanceConnectionState -- initiate or execute the current
+ * meta command, and return the next state to set.
  *
- * Returns an updated timestamp from 'now', used to update 'now' at callsite.
+ * *now is updated to the current time, unless the command is expected to
+ * take no time to execute.
  */
-static instr_time
-doExecuteCommand(TState *thread, CState *st, instr_time now)
+static ConnectionStateEnum
+executeMetaCommand(TState *thread, CState *st, instr_time *now)
 {
    Command    *command = sql_script[st->use_file].commands[st->command];
+   int         argc;
+   char      **argv;
+
+   Assert(command != NULL && command->type == META_COMMAND);
 
-   /* execute the command */
-   if (command->type == SQL_COMMAND)
+   argc = command->argc;
+   argv = command->argv;
+
+   if (debug)
    {
-       if (!sendCommand(st, command))
+       fprintf(stderr, "client %d executing \\%s", st->id, argv[0]);
+       for (int i = 1; i < argc; i++)
+           fprintf(stderr, " %s", argv[i]);
+       fprintf(stderr, "\n");
+   }
+
+   if (command->meta == META_SLEEP)
+   {
+       int         usec;
+
+       /*
+        * A \sleep doesn't execute anything, we just get the delay from the
+        * argument, and enter the CSTATE_SLEEP state.  (The per-command
+        * latency will be recorded in CSTATE_SLEEP state, not here, after the
+        * delay has elapsed.)
+        */
+       if (!evaluateSleep(st, argc, argv, &usec))
        {
-           commandFailed(st, "SQL", "SQL command send failed");
-           st->state = CSTATE_ABORTED;
+           commandFailed(st, "sleep", "execution of meta-command failed");
+           return CSTATE_ABORTED;
        }
-       else
-           st->state = CSTATE_WAIT_RESULT;
+
+       INSTR_TIME_SET_CURRENT_LAZY(*now);
+       st->sleep_until = INSTR_TIME_GET_MICROSEC(*now) + usec;
+       return CSTATE_SLEEP;
    }
-   else if (command->type == META_COMMAND)
+   else if (command->meta == META_SET)
    {
-       int         argc = command->argc;
-       char      **argv = command->argv;
+       PgBenchExpr *expr = command->expr;
+       PgBenchValue result;
 
-       if (debug)
+       if (!evaluateExpr(thread, st, expr, &result))
        {
-           fprintf(stderr, "client %d executing \\%s",
-                   st->id, argv[0]);
-           for (int i = 1; i < argc; i++)
-               fprintf(stderr, " %s", argv[i]);
-           fprintf(stderr, "\n");
+           commandFailed(st, argv[0], "evaluation of meta-command failed");
+           return CSTATE_ABORTED;
        }
 
-       if (command->meta == META_SLEEP)
+       if (!putVariableValue(st, argv[0], argv[1], &result))
        {
-           int         usec;
-
-           /*
-            * A \sleep doesn't execute anything, we just get the delay from
-            * the argument, and enter the CSTATE_SLEEP state.  (The
-            * per-command latency will be recorded in CSTATE_SLEEP state, not
-            * here, after the delay has elapsed.)
-            */
-           if (!evaluateSleep(st, argc, argv, &usec))
-           {
-               commandFailed(st, "sleep", "execution of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
-
-           INSTR_TIME_SET_CURRENT_LAZY(now);
-
-           st->sleep_until = INSTR_TIME_GET_MICROSEC(now) + usec;
-           st->state = CSTATE_SLEEP;
-           return now;
+           commandFailed(st, "set", "assignment of meta-command failed");
+           return CSTATE_ABORTED;
        }
-       else if (command->meta == META_SET)
-       {
-           PgBenchExpr *expr = command->expr;
-           PgBenchValue result;
-
-           if (!evaluateExpr(thread, st, expr, &result))
-           {
-               commandFailed(st, argv[0], "evaluation of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
+   }
+   else if (command->meta == META_IF)
+   {
+       /* backslash commands with an expression to evaluate */
+       PgBenchExpr *expr = command->expr;
+       PgBenchValue result;
+       bool        cond;
 
-           if (!putVariableValue(st, argv[0], argv[1], &result))
-           {
-               commandFailed(st, "set", "assignment of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
-       }
-       else if (command->meta == META_IF)
+       if (!evaluateExpr(thread, st, expr, &result))
        {
-           /* backslash commands with an expression to evaluate */
-           PgBenchExpr *expr = command->expr;
-           PgBenchValue result;
-           bool        cond;
-
-           if (!evaluateExpr(thread, st, expr, &result))
-           {
-               commandFailed(st, argv[0], "evaluation of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
-
-           cond = valueTruth(&result);
-           conditional_stack_push(st->cstack, cond ? IFSTATE_TRUE : IFSTATE_FALSE);
+           commandFailed(st, argv[0], "evaluation of meta-command failed");
+           return CSTATE_ABORTED;
        }
-       else if (command->meta == META_ELIF)
-       {
-           /* backslash commands with an expression to evaluate */
-           PgBenchExpr *expr = command->expr;
-           PgBenchValue result;
-           bool        cond;
 
-           if (conditional_stack_peek(st->cstack) == IFSTATE_TRUE)
-           {
-               /*
-                * elif after executed block, skip eval and wait for endif.
-                */
-               conditional_stack_poke(st->cstack, IFSTATE_IGNORED);
-               st->state = CSTATE_END_COMMAND;
-               return now;
-           }
-
-           if (!evaluateExpr(thread, st, expr, &result))
-           {
-               commandFailed(st, argv[0], "evaluation of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
+       cond = valueTruth(&result);
+       conditional_stack_push(st->cstack, cond ? IFSTATE_TRUE : IFSTATE_FALSE);
+   }
+   else if (command->meta == META_ELIF)
+   {
+       /* backslash commands with an expression to evaluate */
+       PgBenchExpr *expr = command->expr;
+       PgBenchValue result;
+       bool        cond;
 
-           cond = valueTruth(&result);
-           Assert(conditional_stack_peek(st->cstack) == IFSTATE_FALSE);
-           conditional_stack_poke(st->cstack, cond ? IFSTATE_TRUE : IFSTATE_FALSE);
+       if (conditional_stack_peek(st->cstack) == IFSTATE_TRUE)
+       {
+           /* elif after executed block, skip eval and wait for endif. */
+           conditional_stack_poke(st->cstack, IFSTATE_IGNORED);
+           return CSTATE_END_COMMAND;
        }
-       else if (command->meta == META_ELSE)
+
+       if (!evaluateExpr(thread, st, expr, &result))
        {
-           switch (conditional_stack_peek(st->cstack))
-           {
-               case IFSTATE_TRUE:
-                   conditional_stack_poke(st->cstack, IFSTATE_ELSE_FALSE);
-                   break;
-               case IFSTATE_FALSE: /* inconsistent if active */
-               case IFSTATE_IGNORED:   /* inconsistent if active */
-               case IFSTATE_NONE:  /* else without if */
-               case IFSTATE_ELSE_TRUE: /* else after else */
-               case IFSTATE_ELSE_FALSE:    /* else after else */
-               default:
-                   /* dead code if conditional check is ok */
-                   Assert(false);
-           }
+           commandFailed(st, argv[0], "evaluation of meta-command failed");
+           return CSTATE_ABORTED;
        }
-       else if (command->meta == META_ENDIF)
+
+       cond = valueTruth(&result);
+       Assert(conditional_stack_peek(st->cstack) == IFSTATE_FALSE);
+       conditional_stack_poke(st->cstack, cond ? IFSTATE_TRUE : IFSTATE_FALSE);
+   }
+   else if (command->meta == META_ELSE)
+   {
+       switch (conditional_stack_peek(st->cstack))
        {
-           Assert(!conditional_stack_empty(st->cstack));
-           conditional_stack_pop(st->cstack);
+           case IFSTATE_TRUE:
+               conditional_stack_poke(st->cstack, IFSTATE_ELSE_FALSE);
+               break;
+           case IFSTATE_FALSE: /* inconsistent if active */
+           case IFSTATE_IGNORED:   /* inconsistent if active */
+           case IFSTATE_NONE:  /* else without if */
+           case IFSTATE_ELSE_TRUE: /* else after else */
+           case IFSTATE_ELSE_FALSE:    /* else after else */
+           default:
+               /* dead code if conditional check is ok */
+               Assert(false);
        }
-       else if (command->meta == META_SETSHELL)
+   }
+   else if (command->meta == META_ENDIF)
+   {
+       Assert(!conditional_stack_empty(st->cstack));
+       conditional_stack_pop(st->cstack);
+   }
+   else if (command->meta == META_SETSHELL)
+   {
+       if (!runShellCommand(st, argv[1], argv + 2, argc - 2))
        {
-           if (!runShellCommand(st, argv[1], argv + 2, argc - 2))
-           {
-               commandFailed(st, "setshell", "execution of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
+           commandFailed(st, "setshell", "execution of meta-command failed");
+           return CSTATE_ABORTED;
        }
-       else if (command->meta == META_SHELL)
+   }
+   else if (command->meta == META_SHELL)
+   {
+       if (!runShellCommand(st, NULL, argv + 1, argc - 1))
        {
-           if (!runShellCommand(st, NULL, argv + 1, argc - 1))
-           {
-               commandFailed(st, "shell", "execution of meta-command failed");
-               st->state = CSTATE_ABORTED;
-               return now;
-           }
+           commandFailed(st, "shell", "execution of meta-command failed");
+           return CSTATE_ABORTED;
        }
-
-       /*
-        * executing the expression or shell command might have taken a
-        * non-negligible amount of time, so reset 'now'
-        */
-       INSTR_TIME_SET_ZERO(now);
-
-       st->state = CSTATE_END_COMMAND;
    }
 
-   return now;
+   /*
+    * executing the expression or shell command might have taken a
+    * non-negligible amount of time, so reset 'now'
+    */
+   INSTR_TIME_SET_ZERO(*now);
+
+   return CSTATE_END_COMMAND;
 }
 
 /*
@@ -4281,6 +4283,7 @@ free_command(Command *command)
        pg_free(command->argv[i]);
    if (command->varprefix)
        pg_free(command->varprefix);
+
    /*
     * It should also free expr recursively, but this is currently not needed
     * as only gset commands (which do not have an expression) are freed.