Fix GH-195: Fix JSON reporter to produce valid JSON

sysbench.report_json() now generates well-formed JSON without comma after the last array element and with proper opening/closing brackets.
akopytov · Apr 1, 2018 · 56b473f · 56b473f
1 parent b316938
commit 56b473f
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 78 deletions.
diff --git a/src/lua/internal/sysbench.lua b/src/lua/internal/sysbench.lua
@@ -88,27 +88,37 @@ function sysbench.report_csv(stat)
    ))
 end
 
--- Report statistics in the CSV format. Add the following to your
+-- Report statistics in the JSON format. Add the following to your
 -- script to replace the default human-readable reports
 --
 -- sysbench.hooks.report_intermediate = sysbench.report_json
 function sysbench.report_json(stat)
+   if not gobj then
+      io.write('[\n')
+      -- hack to print the closing bracket when the Lua state of the reporting
+      -- thread is closed
+      gobj = newproxy(true)
+      getmetatable(gobj).__gc = function () io.write('\n]\n') end
+   else
+      io.write(',\n')
+   end
+
    local seconds = stat.time_interval
-   print(string.format([[
-{
-  "time": %4.0f,
-  "threads": %u,
-  "tps": %4.2f,
-  "qps": {
-    "total": %4.2f,
-    "reads": %4.2f,
-    "writes": %4.2f,
-    "other": %4.2f
-  },
-  "latency": %4.2f,
-  "errors": %4.2f,
-  "reconnects": %4.2f
-},]],
+   io.write(([[
+  {
+    "time": %4.0f,
+    "threads": %u,
+    "tps": %4.2f,
+    "qps": {
+      "total": %4.2f,
+      "reads": %4.2f,
+      "writes": %4.2f,
+      "other": %4.2f
+    },
+    "latency": %4.2f,
+    "errors": %4.2f,
+    "reconnects": %4.2f
+  }]]):format(
             stat.time_total,
             stat.threads_running,
             stat.events / seconds,

diff --git a/src/sb_lua.c b/src/sb_lua.c
@@ -1635,7 +1635,10 @@ int sb_lua_report_thread_init(void)
   return 0;
 }
 
-void sb_lua_report_thread_done(void)
+void sb_lua_report_thread_done(void *arg)
 {
-  sb_lua_close_state(tls_lua_ctxt.L);
+  (void) arg; /* unused */
+
+  if (sb_lua_loaded())
+    sb_lua_close_state(tls_lua_ctxt.L);
 }
diff --git a/src/sb_lua.h b/src/sb_lua.h
@@ -1,5 +1,5 @@
 /* Copyright (C) 2006 MySQL AB
-   Copyright (C) 2006-2017 Alexey Kopytov <[email protected]>
+   Copyright (C) 2006-2018 Alexey Kopytov <[email protected]>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -37,6 +37,6 @@ int sb_lua_call_custom_command(const char *name);
 
 int sb_lua_report_thread_init(void);
 
-void sb_lua_report_thread_done(void);
+void sb_lua_report_thread_done(void *);
 
 bool sb_lua_loaded(void);
diff --git a/src/sysbench.c b/src/sysbench.c
@@ -220,9 +220,7 @@ static void report_intermediate(void)
     silence intermediate reports at the end of the test
   */
   if (ck_pr_load_uint(&sb_globals.report_interval) == 0)
-  {
     return;
-  }
 
   sb_counters_agg_intermediate(cnt);
   report_get_common_stat(&stat, cnt);
@@ -914,9 +912,9 @@ static void *report_thread_proc(void *arg)
   sb_rand_thread_init();
 
   if (sb_lua_loaded() && sb_lua_report_thread_init())
-  {
     return NULL;
-  }
+
+  pthread_cleanup_push(sb_lua_report_thread_done, NULL);
 
   log_text(LOG_DEBUG, "Reporting thread started");
 
@@ -944,6 +942,8 @@ static void *report_thread_proc(void *arg)
     pause_ns = next_ns - curr_ns;
   }
 
+  pthread_cleanup_pop(1);
+
   return NULL;
 }
 
@@ -963,9 +963,9 @@ static void *checkpoints_thread_proc(void *arg)
   sb_rand_thread_init();
 
   if (sb_lua_loaded() && sb_lua_report_thread_init())
-  {
     return NULL;
-  }
+
+  pthread_cleanup_push(sb_lua_report_thread_done, NULL);
 
   log_text(LOG_DEBUG, "Checkpoints report thread started");
 
@@ -990,8 +990,7 @@ static void *checkpoints_thread_proc(void *arg)
     report_cumulative();
   }
 
-  if (sb_lua_loaded())
-    sb_lua_report_thread_done();
+  pthread_cleanup_pop(1);
 
   return NULL;
 }
@@ -1149,26 +1148,6 @@ static int run_test(sb_test_t *test)
   if (test->ops.cleanup != NULL && test->ops.cleanup() != 0)
     return 1;
 
-  /* print test-specific stats */
-  if (!sb_globals.error)
-  {
-    if (sb_globals.histogram)
-    {
-      log_text(LOG_NOTICE, "Latency histogram (values are in milliseconds)");
-      sb_histogram_print(&sb_latency_histogram);
-      log_text(LOG_NOTICE, " ");
-    }
-
-    report_cumulative();
-  }
-
-  pthread_mutex_destroy(&sb_globals.exec_mutex);
-
-  /* finalize test */
-  if (test->ops.done != NULL)
-    (*(test->ops.done))();
-
-  /* Delay killing the reporting threads to avoid mutex lock leaks */
   if (report_thread_created)
   {
     if (sb_thread_cancel(report_thread) || sb_thread_join(report_thread, NULL))
@@ -1189,6 +1168,25 @@ static int run_test(sb_test_t *test)
       log_errno(LOG_FATAL, "Terminating the checkpoint thread failed.");
   }
 
+  /* print test-specific stats */
+  if (!sb_globals.error)
+  {
+    if (sb_globals.histogram)
+    {
+      log_text(LOG_NOTICE, "Latency histogram (values are in milliseconds)");
+      sb_histogram_print(&sb_latency_histogram);
+      log_text(LOG_NOTICE, " ");
+    }
+
+    report_cumulative();
+  }
+
+  pthread_mutex_destroy(&sb_globals.exec_mutex);
+
+  /* finalize test */
+  if (test->ops.done != NULL)
+    (*(test->ops.done))();
+
   return sb_globals.error != 0;
 }
 

diff --git a/tests/t/api_reports.t b/tests/t/api_reports.t
@@ -3,7 +3,7 @@ Tests for custom report hooks
 ########################################################################
 
 # Trigger one intermediate and one cumulative report
-  $ SB_ARGS="api_reports.lua --time=3 --report-interval=2 --verbosity=1"
+  $ SB_ARGS="api_reports.lua --time=5 --report-interval=2 --verbosity=1"
 
 ########################################################################
 # Default human-readable format via a custom hook
@@ -22,7 +22,8 @@ Tests for custom report hooks
 
   $ sysbench $SB_ARGS run
   \[ 2s \] thds: 1 tps: [0-9]*\.[0-9]* qps: 0\.00 \(r\/w\/o: 0\.00\/0\.00\/0\.00\) lat \(ms,95%\): [1-9][0-9]*\.[0-9]* err\/s 0\.00 reconn\/s: 0\.00 (re)
-  \[ 3s \] thds: 0 tps: [0-9]*\.[0-9]* qps: 0\.00 \(r\/w\/o: 0\.00\/0\.00\/0\.00\) lat \(ms,95%\): [1-9][0-9]*\.[0-9]* err\/s 0\.00 reconn\/s: 0\.00 (re)
+  \[ 4s \] thds: 1 tps: [0-9]*\.[0-9]* qps: 0\.00 \(r\/w\/o: 0\.00\/0\.00\/0\.00\) lat \(ms,95%\): [1-9][0-9]*\.[0-9]* err\/s 0\.00 reconn\/s: 0\.00 (re)
+  \[ 5s \] thds: 0 tps: [0-9]*\.[0-9]* qps: 0\.00 \(r\/w\/o: 0\.00\/0\.00\/0\.00\) lat \(ms,95%\): [1-9][0-9]*\.[0-9]* err\/s 0\.00 reconn\/s: 0\.00 (re)
 
 ########################################################################
 # CSV format via a custom hook
@@ -41,7 +42,8 @@ Tests for custom report hooks
 
   $ sysbench $SB_ARGS run
   2,1,[0-9]*\.[0-9]*,0\.00,0\.00,0\.00,0\.00,[1-9][0-9]*\.[0-9]*,0\.00,0\.00 (re)
-  3,0,[0-9]*\.[0-9]*,0\.00,0\.00,0\.00,0\.00,[1-9][0-9]*\.[0-9]*,0\.00,0\.00 (re)
+  4,1,[0-9]*\.[0-9]*,0\.00,0\.00,0\.00,0\.00,[1-9][0-9]*\.[0-9]*,0\.00,0\.00 (re)
+  5,0,[0-9]*\.[0-9]*,0\.00,0\.00,0\.00,0\.00,[1-9][0-9]*\.[0-9]*,0\.00,0\.00 (re)
 
 ########################################################################
 # JSON format via a custom hook
@@ -59,31 +61,49 @@ Tests for custom report hooks
   > EOF
 
   $ sysbench $SB_ARGS run
-  {
-    "time":    2,
-    "threads": 1,
-    "tps": *.*, (glob)
-    "qps": {
-      "total": 0.00,
-      "reads": 0.00,
-      "writes": 0.00,
-      "other": 0.00
+  [
+    {
+      "time":    2,
+      "threads": 1,
+      "tps": *.*, (glob)
+      "qps": {
+        "total": 0.00,
+        "reads": 0.00,
+        "writes": 0.00,
+        "other": 0.00
+      },
+      "latency": [1-9][0-9]*\.[0-9]*, (re)
+      "errors": 0.00,
+      "reconnects": 0.00
     },
-    "latency": [1-9][0-9]*\.[0-9]*, (re)
-    "errors": 0.00,
-    "reconnects": 0.00
-  },
-  {
-    "time":    3,
-    "threads": 0,
-    "tps": *.*, (glob)
-    "qps": {
-      "total": 0.00,
-      "reads": 0.00,
-      "writes": 0.00,
-      "other": 0.00
-    },
-    "latency": [1-9][0-9]*\.[0-9]*, (re)
-    "errors": 0.00,
-    "reconnects": 0.00
-  },
+    {
+      "time":    4,
+      "threads": 1,
+      "tps": *.*, (glob)
+      "qps": {
+        "total": 0.00,
+        "reads": 0.00,
+        "writes": 0.00,
+        "other": 0.00
+      },
+      "latency": [1-9][0-9]*\.[0-9]*, (re)
+      "errors": 0.00,
+      "reconnects": 0.00
+    }
+  ]
+  [
+    {
+      "time":    5,
+      "threads": 0,
+      "tps": *.*, (glob)
+      "qps": {
+        "total": 0.00,
+        "reads": 0.00,
+        "writes": 0.00,
+        "other": 0.00
+      },
+      "latency": [1-9][0-9]*\.[0-9]*, (re)
+      "errors": 0.00,
+      "reconnects": 0.00
+    }
+  ]