<div dir="ltr">On Tue, May 21, 2013 at 7:05 AM, Jeff Darcy <span dir="ltr"><<a href="mailto:jdarcy@redhat.com" target="_blank">jdarcy@redhat.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 05/21/2013 09:10 AM, Pranith Kumar Karampuri wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
scenario-1 won't happen because there exists a chance for it to acquire<br>
truncate's full file lock after any 128k range sync happens.<br>
<br>
Scenario-2 won't happen because extra self-heals that are launched on the<br>
same file will be blocked in self-heal-domain so the data-path's locks are<br>
not affected by this.<br>
</blockquote>
<br></div>
This is two changes for two scenarios:<br>
<br>
* Change granular-lock order to avoid scenario 1.<br>
<br>
* Add a new lock to avoid scenario 2.<br>
<br>
I'm totally fine with the second part, but the first part makes me a bit uneasy. As I recall, the "chained" locking (lock second range before unlocking the first) was a deliberate choice to avoid windows where somebody could jump in with a truncate during self-heal. It certainly wasn't the obvious thing to do, suggesting there was a specific reason. </blockquote>
<div><br></div><div style>Chained locking really was to avoid the start of a second self-heal while one is in progress. By giving up the big lock (after holding the small lock), regional operations can progress but the second self-heal waiting to hold the big lock is still held off. Truncating isn't really an issue as long as the truncate transaction locks from new EOF till infinity (which it is) and self-heal will not race in those regions. Of course, with chained locking, full-file truncates can starve.</div>
<div style><br></div><div style>It's not obvious what use case Pranith is facing where self-heal and ftruncate() are competing. Is it just an artificial/hand-crafted test case, or a real-life access pattern?</div><div>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Until we recall what that reason was I don't think we can determine whether this is a bug or a feature. If it's a feature, or if we don't know, this change is likely to break something instead of fixing it.</blockquote>
<div><br></div><div style>The sole reason was to prevent the start of a second self-heal. Having self-heals serialize in a separate domain solves the problem (except if we are looking at runtime compatibility across multiple versions in this scenario.. aargh!)<br>
</div><div style><br></div><div style>Avati</div></div></div></div>