We cannot afford to acknowledge the user that the configuration is changed before actually making those changes on stable media. If it takes longer with O_SYNC and fsync(dir), it just does. I do see the benefits of reduced regression run test times without the O_SYNC/fsync. But I don't think just that justifies removing O_SYNC / fsync(dir). It would be ridiculous for a filesystem's management software to not follow the safety requirements of storing data on a filesystem :-). Let's just work on making the scripts more efficient and intelligent (removing unnecessary sleeps, reducing iterations or file counts which may be unnecessarily large etc.) for bringing down runtime.<div>
<br></div><div>Avati<br><br><div class="gmail_quote">On Wed, Feb 6, 2013 at 5:31 AM, Pranith Kumar Karampuri <span dir="ltr"><<a href="mailto:pkarampu@redhat.com" target="_blank">pkarampu@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hi,<br>
Here is the comparison of command execution times of 'volume set' on v3.3.0 and master:<br>
<br>
On master:<br>
⚡ time gluster volume set r2 performance.read-ahead off<br>
volume set: success<br>
<br>
real 0m1.250s<br>
user 0m0.059s<br>
sys 0m0.019s<br>
<br>
On v3.3.0:<br>
⚡ time gluster volume set r2 performance.read-ahead off<br>
Set volume successful<br>
<br>
real 0m0.081s<br>
user 0m0.051s<br>
sys 0m0.016s<br>
<br>
The contributing factors to the delay are opening files in gluster-store with O_SYNC and fsync of the directory fd. With the following changes, the execution times came down to:<br>
<br>
⚡ time gluster volume set r2 read-ahead off<br>
volume set: success<br>
<br>
real 0m0.130s<br>
user 0m0.063s<br>
sys 0m0.019s<br>
<br>
⚡ g d<br>
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c<br>
index 863b70c..a225dfd 100644<br>
--- a/xlators/mgmt/glusterd/src/glusterd-store.c<br>
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c<br>
@@ -89,7 +89,7 @@ glusterd_store_mkstemp (glusterd_store_handle_t *shandle)<br>
GF_ASSERT (shandle->path);<br>
<br>
snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);<br>
- fd = open (tmppath, O_RDWR | O_CREAT | O_TRUNC | O_SYNC, 0600);<br>
+ fd = open (tmppath, O_RDWR | O_CREAT | O_TRUNC, 0600);<br>
if (fd <= 0) {<br>
gf_log (THIS->name, GF_LOG_ERROR, "Failed to open %s, "<br>
"error: %s", tmppath, strerror (errno));<br>
@@ -121,12 +121,12 @@ glusterd_store_sync_direntry (char *path)<br>
goto out;<br>
}<br>
<br>
- ret = fsync (dirfd);<br>
- if (ret) {<br>
- gf_log (this->name, GF_LOG_ERROR, "Failed to fsync %s, due to "<br>
- "%s", pdir, strerror (errno));<br>
- goto out;<br>
- }<br>
+ //ret = fsync (dirfd);<br>
+ //if (ret) {<br>
+ // gf_log (this->name, GF_LOG_ERROR, "Failed to fsync %s, due to "<br>
+ // "%s", pdir, strerror (errno));<br>
+ // goto out;<br>
+ //}<br>
<br>
ret = 0;<br>
out:<br>
<br>
run-tests.sh executed in 20 minutes with this change as opposed to 30 minutes without this change on my machine.<br>
If we remove the O_SYNC flag alone and keep fsync of dir as is, here is the execution time:<br>
<br>
⚡ time gluster volume set r2 read-ahead off<br>
volume set: success<br>
<br>
real 0m0.653s<br>
user 0m0.054s<br>
sys 0m0.015s<br>
<br>
Do you feel we need to change anything based on these execution time differences?<br>
It will save quite a bit of time at least for gerrit builds if there is a way to turn them off.<br>
Let me know your views.<br>
<br>
Pranith.<br>
<br>
_______________________________________________<br>
Gluster-devel mailing list<br>
<a href="mailto:Gluster-devel@nongnu.org">Gluster-devel@nongnu.org</a><br>
<a href="https://lists.nongnu.org/mailman/listinfo/gluster-devel" target="_blank">https://lists.nongnu.org/mailman/listinfo/gluster-devel</a><br>
</blockquote></div><br></div>