<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 01/09/2020 02:58, NeilBrown wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <pre class="moz-quote-pre" wrap="">On Mon, Aug 31 2020, <a class="moz-txt-link-abbreviated" href="mailto:quentin.bouget@cea.fr">quentin.bouget@cea.fr</a> wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">On 31/08/2020 06:03, NeilBrown wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">I tried removing that check, but there is a test (hsm_test 108)
which checks for repeating identical intervals.
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
test108 handles a "growing" extent (offset=0, length=1, 2, 3, ...).
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
test108_progress() in llapi_hsm_test.c sets:
                he.offset = 0;
                he.length = length;
where 'length' is 1000.  It reports progress for this extent 1000 times.
Then complains if the total progress isn't 1000.</pre>
    </blockquote>
    <p>My bad, I read that wrong. You are right.<br>
      <br>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">test112 handles acknowledging non-overlapping extents twice each.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">Yep.  10 non-overlapping extents, then sends the same 10 again.

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
I wrote a test to check what happens if you acknowledge overlapping extents:

  * (offset=0, length=256)
  * (offset=128, length=256)

And surely enough mdt_cdt_get_work_done() returns "512" rather than the 
expected "384" (ie. 128 + 256).

Even worse, when acknowledging a "shrinking" extent (offset=0, length=N, 
N - 1, N - 2, ...), only the last value is kept in store.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Does that even mean anything?  It seems to be saying "I've done N amount
of work" and then "Sorry, I lied, I've only done N-1"...</pre>
    </blockquote>
    <p>I think it could happen if a copytool spreads HSM actions to
      multiple workers, one of the worker stalls, its work is
      resubmitted to another worker, and both of them acknowledge
      extents with different lengths.</p>
    <p>But yes, this is at best a weird/bad corner case.<br>
      <br>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <pre class="moz-quote-pre" wrap="">But I'm surprised at your result that only the last is kept.  Reading
the code strongly suggests that it would report the sum of all these
regions.  N^2/2 if you continued to  N-N.</pre>
    </blockquote>
    <p>That was me being wrong again. It behaves just like you say.<br>
      <br>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <pre class="moz-quote-pre" wrap="">How do you perform these tests?  Is there a command-line tool, or would
I need to write some code?</pre>
    </blockquote>
    <p>I copied test112 in llapi_hsm_test.c and edited the code (and
      read the error message wrong =/).<br>
      <br>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap=""><strike> From this, I think that exact duplicates are not really ignored, 
rather, intervals that share the same starting point overwrite one 
another, until only the last one remains.
Bug or feature? I don't really know.</strike>


</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">I want to clean up this code as I'm converting all users of the lustre
interval-tree to use the upstream-linux interval tree code.  What should
I do?
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
How does the upstream-linux interval tree compares to Lustre's?
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
The interesting difference is that the lustre interval-tree refuses to
store exact duplicates (same start, same end), while the Linux
interval-tree will accept any new interval.
I think this made some sense for the first user for interval-trees in
lustre, which was LDLM extent locking, but some other users need to
jump through hoops to handle duplicates correctly.

For hsm_update_work(), it just tries to store the interval it was given
and if that fails, it say "oh well, too bad".</pre>
    </blockquote>
    <p>Yes, there is a comment saying : "it can happen if ct sends 2
      times the same progress".</p>
    <p>From "git blame", this seems to be coming from one of the
      earliest HSM commits, so I don't know if this was ever useful in
      practice or just an instance of defensive programming.<br>
      <br>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">If their behaviours match, there should not be any issue (so far as the 
current behaviour can be considered issue-free).
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Certainly I could preserve exactly the same behaviour, but I find it
hard to write code that doesn't make any sense.
If I *were* to keep exactly the same behaviour as current, I wouldn't use
an interval tree, as (if I understand it correctly), the intervals
aren't really relevant.  It just stores discrete "start+length"
pairs and rejects duplicates.  I'd probably use an rhashtable for that.</pre>
    </blockquote>
    <p>Assuming duplicated extents, but otherwise non-overlapping, I
      agree.</p>
    <p>Ideally, there would be a standard data structure that
      automatically merges overlapping/contiguous extents.<br>
      But I don't know any, and I don't think it is worth coding one
      from scratch.<br>
      <br>
    </p>
    <p>
    </p>
    <blockquote type="cite"
      cite="mid:87mu2aqpnp.fsf@notabene.neil.brown.name">
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">Otherwise, I think it would be OK to just assume sending overlapping 
extents is a programming error and the server does not need to protect 
itself against it.
In terms of security, this isn't very good, but the problem already 
exists, and copytools are supposedly trusted binaries run as root.
You could then remove test108 as it is itself a programming error, 
test112 as well, and maybe others.

Just to be sure, you could open a new issue on Jira, and let others rule 
how much of a bug/feature the whole thing is.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
I guess the key question here is: who are those "others" ??
Jira has a "Component/s" field, but it seems to be fixed at "None".
How would I ensure that the Jira issue got to the right people?</pre>
    </blockquote>
    <p>There is a "label" field that can take "HSM" as a value.<br>
    </p>
    <p>Other than that, I agree with Ben that removing the feature would
      be the easiest way to fix all this.<br>
      I am just worried that someone somewhere uses it. For example, I
      can imagine a monitoring script that polls the coordinator's
      active request procfile to detect stalled HSM actions. <br>
    </p>
    <p>I'll check with our admins, but I don't think we use it at CEA.
      Maybe if Cray & DDN confirm that they don't know anyone using
      it, it will be enough to remove the code, like Ben suggests. Or
      maybe we axe the feature and wait to see if anyone complains.<br>
    </p>
    <p>From the repos linked by AurĂ©lien, every copytool seems to mind
      the constraint of not sending overlapping extents (not even
      duplicates). I also have access to the sources of a different
      copytool that uses llapi_hsm_action_progress() as a heartbeat
      (offset=0, length=0).<br>
    </p>
    <p>Sorry about the earlier mistakes.</p>
    <p>Have a good day,<br>
      Quentin<br>
    </p>
  </body>
</html>