Skip to contentSkip to conversation

suggestion for fio --client to remove garbled output

PATCH Ben England 4 participants 6 messages Feb 15, 2015 – Feb 27, 2015 4 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

This patch makes fio --client JSON output work better when many server threads are used by removing interleaved output from fio servers with --output-format=json .

Background: Red Hat is testing large numbers of OpenStack guests with Cinder block devices backed by Ceph storage using fio. Originally we were using ssh to start independent fio processes on the guests, but we ran into problems with threads not starting and stopping at the same time, despite using options to use fixed elapsed time for all threads. But when we started using --client, we noticed that thread time skew is greatly reduced, because fio is *already running* on the server and we only have to connect to it and start the test. This is great, but for large numbers of fio servers, it becomes impossible to parse the output.

For example, here's a sample of per-VM JSON output:

Disk stats (read/write):
<vm4> {

<vm4>   "fio version" : "fio-2.2.4",
<vm4>   "timestamp" : 1421677173,
<vm4>   "time" : "Mon Jan 19 09:19:33 2015",
<vm4>   "jobs" : [

<vm4>
<vm4> ],
<vm4> "disk_util" : [
...
<vm4> ]
<vm4> }

https://s3.amazonaws.com/ben.england/client.c.diff
https://s3.amazonaws.com/ben.england/fio-results.2015-01-20-11-50.json.gz

The above output works fine for a few threads and is not hard to filter out, but the various guests' output sections get mixed together for large thread counts such as 256 client threads. if you look at the previous URL fio-results*json.gz containing garbled output, the cause of this output is the handle_text() routine, which is called by fio whenever a FIO_NET_CMD_TEXT PDU is received by the fio client (in fio_handle_client()). If you use --debug=all you can see that these PDUs are processed in whatever order they are received from the fio server processes, essentially in random order. But almost all of this info is in the final json object output at the end of the file (except for the timestamp field which I wish was there!) so there is no reason to output twice, except as a debugging tool perhaps. The patch is to V2.2.4.

[ben@localhost fio-master]$ diff -u client.{c.sav2,c}

client.c renamed from client.c.sav2 +0 -6
@@ -968,12 +968,6 @@
 	const char *name;
 	int fio_unused ret;
 
-	name = client->name ? client->name : client->hostname;
-
-	if (!client->skip_newline)
-		fprintf(f_out, "<%s> ", name);
-	ret = fwrite(buf, pdu->buf_len, 1, f_out);
-	fflush(f_out);
 	client->skip_newline = strchr(buf, '\n') == NULL;
 }
 

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

suggestion: patch to make per-thread IOPS more accurate

The following small patch to stat.c (using fio 2.2.4 from github) outputs IOPS field in JSON format as a floating point number instead of an integer. This improves accuracy in case where fio --client runs use lots of threads with single-digit IOPS per thread. It seems to work, here's a snippet of output from a short fio run with rate_iops=10 in the job file.

     "write" : {
        "io_bytes" : 6464,
        "bw" : 646,
        "iops" : 10.10,
        "runtime" : 10000,

Why the patch: IOPS number is rounded to integer in stats.c calls to num2str(). This doesn't sound like much of a problem because in many use cases, with large IOPS number the error is negligible. But in this use case where we have many threads (we would like to get into the thousands eventually), the IOPS/thread may be quite low and integer rounding can introduce significant error. For example, if we are doing 5,000 IOPS over 1,000 threads, average throughput is 5 IOPS and potential error is ~20%, but some threads could have much higher error in IOPS because of integer format.

background: fio's --client option could prove *extremely* useful for work where we inject I/O workload from tens, hundreds or even thousands of VMs to an OpenStack, container or other virtualization environment. I really like this feature combined with "fio --server --daemonize=/var/run/fio-svr.pid" command run on workload generators, because it means that we don't have to use ssh/pdsh to start up a connection and launch the workload generator on each host. ssh is problematic for this, I have to throttle it because it locks up if you have too many concurrent ssh sessions starting up. pdsh is better but still has limits because you actually have to start each thread up from scratch, sometimes in competition with the workload you want to measure.

stat.c renamed from stat.c.sav +4 -3
@@ -674,7 +674,8 @@
 		struct group_run_stats *rs, int ddir, struct json_object *parent)
 {
 	unsigned long min, max;
-	unsigned long long bw, iops;
+	unsigned long long bw;
+	double iops;
 	unsigned int *ovals = NULL;
 	double mean, dev;
 	unsigned int len, minv, maxv;
@@ -698,12 +699,12 @@
 		uint64_t runt = ts->runtime[ddir];
 
 		bw = ((1000 * ts->io_bytes[ddir]) / runt) / 1024;
-		iops = (1000 * (uint64_t) ts->total_io_u[ddir]) / runt;
+		iops = (1000.0 * (uint64_t) ts->total_io_u[ddir]) / runt;
 	}
 
 	json_object_add_value_int(dir_object, "io_bytes", ts->io_bytes[ddir] >> 10);
 	json_object_add_value_int(dir_object, "bw", bw);
-	json_object_add_value_int(dir_object, "iops", iops);
+	json_object_add_value_float(dir_object, "iops", iops);
 	json_object_add_value_int(dir_object, "runtime", ts->runtime[ddir]);
 	json_object_add_value_int(dir_object, "total_ios", ts->total_io_u[ddir]);
 	json_object_add_value_int(dir_object, "short_ios", ts->short_io_u[ddir]);
#6 lore raw

Re: suggestion: patch to make per-thread IOPS more accurate

Hi Ben,

I confess that I'm not using JSON output myself, but I think this would be really beneficial when there are a lot of clients (like VMs).

Jens, would this be an acceptable change? I think I'd appreciate this if I switch over to using the JSON output.

Mark

Quoted 44 lines · Ben England

The following small patch to stat.c (using fio 2.2.4 from github) outputs IOPS field in JSON format as a floating point number instead of an integer. This improves accuracy in case where fio --client runs use lots of threads with single-digit IOPS per thread. It seems to work, here's a snippet of output from a short fio run with rate_iops=10 in the job file.

      "write" : {
         "io_bytes" : 6464,
         "bw" : 646,
         "iops" : 10.10,
         "runtime" : 10000,

Why the patch: IOPS number is rounded to integer in stats.c calls to num2str(). This doesn't sound like much of a problem because in many use cases, with large IOPS number the error is negligible. But in this use case where we have many threads (we would like to get into the thousands eventually), the IOPS/thread may be quite low and integer rounding can introduce significant error. For example, if we are doing 5,000 IOPS over 1,000 threads, average throughput is 5 IOPS and potential error is ~20%, but some threads could have much higher error in IOPS because of integer format.

background: fio's --client option could prove *extremely* useful for work where we inject I/O workload from tens, hundreds or even thousands of VMs to an OpenStack, container or other virtualization environment. I really like this feature combined with "fio --server --daemonize=/var/run/fio-svr.pid" command run on workload generators, because it means that we don't have to use ssh/pdsh to start up a connection and launch the workload generator on each host. ssh is problematic for this, I have to throttle it because it locks up if you have too many concurrent ssh sessions starting up. pdsh is better but still has limits because you actually have to start each thread up from scratch, sometimes in competition with the workload you want to measure.

stat.c renamed from stat.c.sav +4 -4
@@ -674,7 +674,8 @@
  		struct group_run_stats *rs, int ddir, struct json_object *parent)
  {
  	unsigned long min, max;
-	unsigned long long bw, iops;
+	unsigned long long bw;
+	double iops;
  	unsigned int *ovals = NULL;
  	double mean, dev;
  	unsigned int len, minv, maxv;
@@ -698,12 +699,12 @@
  		uint64_t runt = ts->runtime[ddir];
 
  		bw = ((1000 * ts->io_bytes[ddir]) / runt) / 1024;
-		iops = (1000 * (uint64_t) ts->total_io_u[ddir]) / runt;
+		iops = (1000.0 * (uint64_t) ts->total_io_u[ddir]) / runt;
  	}
 
  	json_object_add_value_int(dir_object, "io_bytes", ts->io_bytes[ddir] >> 10);
  	json_object_add_value_int(dir_object, "bw", bw);
-	json_object_add_value_int(dir_object, "iops", iops);
+	json_object_add_value_float(dir_object, "iops", iops);
  	json_object_add_value_int(dir_object, "runtime", ts->runtime[ddir]);
  	json_object_add_value_int(dir_object, "total_ios", ts->total_io_u[ddir]);
  	json_object_add_value_int(dir_object, "short_ios", ts->short_io_u[ddir]);
--

To unsubscribe from this list: send the line "unsubscribe fio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

Re: suggestion: patch to make per-thread IOPS more accurate

Quoted 7 lines · Mark Nelson

Hi Ben,

I confess that I'm not using JSON output myself, but I think this would be really beneficial when there are a lot of clients (like VMs).

Jens, would this be an acceptable change? I think I'd appreciate this if I switch over to using the JSON output.

I like the change, my only worry is that it'll break somebody's parsing... So from that end, perhaps it'd be better to add a new field instead of reusing the 'iops' field to dump it.

Maybe I'm just being overly cautious here, feel free to tell me I'm wrong.

Signature · 1 line
Jens Axboe
#8 lore raw

Re: suggestion: patch to make per-thread IOPS more accurate

----- Original Message -----

Quoted 11 lines

From: "Jens Axboe" <[email protected]>
To: "Mark Nelson" <[email protected]>, "Ben England" <[email protected]>, [email protected]
Sent: Thursday, February 26, 2015 5:20:04 PM
Subject: Re: suggestion: patch to make per-thread IOPS more accurate

Quoted 5 lines · Mark Nelson

Hi Ben,

I confess that I'm not using JSON output myself, but I think this would be really beneficial when there are a lot of clients (like VMs).

Quoted 8 lines
Quoted 2 lines

Jens, would this be an acceptable change? I think I'd appreciate this if I switch over to using the JSON output.

I like the change, my only worry is that it'll break somebody's parsing... So from that end, perhaps it'd be better to add a new field instead of reusing the 'iops' field to dump it.

Maybe I'm just being overly cautious here, feel free to tell me I'm wrong.

Since this is for the json data, I don't think the parsing would be an issue. I have at least tried reading this data with parl::JSON with this patch, and there have been no issues.

-Andrew

Quoted 9 lines

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe fio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw

Re: suggestion: patch to make per-thread IOPS more accurate

Quoted 17 lines · Andrew Theurer

----- Original Message -----

Quoted 14 lines

From: "Jens Axboe" <[email protected]>
To: "Mark Nelson" <[email protected]>, "Ben England" <[email protected]>, [email protected]
Sent: Thursday, February 26, 2015 5:20:04 PM
Subject: Re: suggestion: patch to make per-thread IOPS more accurate

Quoted 7 lines · Mark Nelson

Hi Ben,

I confess that I'm not using JSON output myself, but I think this would be really beneficial when there are a lot of clients (like VMs).

Jens, would this be an acceptable change? I think I'd appreciate this if I switch over to using the JSON output.

Quoted 7 lines
Quoted 5 lines

I like the change, my only worry is that it'll break somebody's parsing... So from that end, perhaps it'd be better to add a new field instead of reusing the 'iops' field to dump it.

Maybe I'm just being overly cautious here, feel free to tell me I'm wrong.

Since this is for the json data, I don't think the parsing would be an issue. I have at least tried reading this data with parl::JSON with this patch, and there have been no issues.

Alright, I was hoping that might be the case. I have applied the patch, thanks guys.

Signature · 1 line
Jens Axboe