ra_serf violating editor API restrictions

Recently, I've been looking into sparse directories compatibility code --
how can a 1.5 client request particular depths of a 1.4 server and get the
right thing. The solution chosen has been a wrapping editor which simply
filters out portions of an editor drive that are outside the requested
depth. With the exception of one theoretical limitation caused by the fact
that editor->delete_entry() is node-kind-ignorant, the results have been
perfect. Well... almost.

I started seeing really odd behavior when running 'svn co --depth
immediates' -- I'd get only a fraction of the tree, and repeated attempts
would yield not even the same fraction. After spending a half-day inside
gdb trying to figure out what was going on, I resorted to the ol' standby:
printf(). What I saw was really surprising to me. It appeared that ra_serf
was driving the update_editor in a manner that completely violated the
depth-first rules of editor driving. Fearing this was a misinteraction with
my new depth-filtering editor, I "unplugged" that code. I saw the same
thing. Confused, I asked Justin about what I saw seeing. Here's the IRC
transcript:

   <jerenkrantz> serf will certainly do out-of-order drives
   <jerenkrantz> that's the whole intent.
   <jerenkrantz> not to be blocked
   <jerenkrantz> there is *no* ordering - the docs at the time and the
                 code i checked in libsvn_wc supported that
      <cmpilato> "Put another way, the producer cannot have two sibling
                 directory batons open at the same time."
   <jerenkrantz> fix the editors then.
   <jerenkrantz> because that's bogus
      <cmpilato> hrm. those editor rules are as old as subversion itself.
   <jerenkrantz> given that it has a parent_baton, it's a silly
                 restriction, imo
      <cmpilato> your opinion restricted, the fact remains that ra_serf
                 violates well-established API restrictions.
      <cmpilato> s/restricted/respected/
                 * cmpilato shouldn't watch TV and type at the same time.
   <jerenkrantz> libsvn_wc had zero problems with it. i asked on list
                 about this way back yonder and no one disagreed
   <jerenkrantz> so, the impl. were happy. hence, rules are bad. =)
      <cmpilato> you merely got lucky.
   <jerenkrantz> it's the only way to do parallel operations
      <cmpilato> no, actually, it isn't.
      <cmpilato> you can parallelize the textdelta stuff -- arguably the
                 expensive part of a checkout.
   <jerenkrantz> nope. that's not possible.
      <cmpilato> whyzat?
   <jerenkrantz> because waiting for the entire report to come along is
                 too slow
   <jerenkrantz> you have to start before the server has given you the
                 complete editor layouts
   <jerenkrantz> err, add-path/etc info
   <jerenkrantz> streaminess...
      <cmpilato> could you be squirreling stuff away into tmpfiles
                 (associated with the open file batons)?
   <jerenkrantz> those files require the dir batons!
   <jerenkrantz> you can't open the textdeltas without them
      <cmpilato> the editor rules say (have always said) that you can
                 open and close directories depth-first, but leave files
                 open until the end of the drive.
   <jerenkrantz> and when this was discussed way back yonder, it was
                 decided that depth-first isn't possible.
   <jerenkrantz> s/feasible/
   <jerenkrantz> and given the code had no problems with it, that was how
                 we proceeded
   <jerenkrantz> doing the directories depth-first means that you only
                 have one directory open at a time. that's bad.
   <jerenkrantz> it makes sense given how ra_neon works; but it's simply
                 not parallelizable without violating that constraint
      <cmpilato> i think i can work around the immediate problems this is
                 causing me -- but i'll need to raise this as an issue
                 with the devlist.
   <jerenkrantz> +1 to discussing on-list

So, we have an interesting situation here. The editor API rules (which are
as old as Subversion itself) are being violated (intentionally, and with
good intent) by ra_serf. Until recently, this hasn't caused any problems
for Subversion's stock editor implementations. And we've talked often in
the past about softening up the editor restrictions. It *is* causing real
problems for my new editor, which does expect a well-behaved editor driver,
but I think I can work around that.

As complex as our working copy code is -- especially now with merge tracking
and sparse directories and such -- and as small as our regression test data
sets are, I fear that it's only a bit of luck that our update editor, status
editor, merge editor, and diff editor all seem to currently be okey dokey
with this asynchronous, parallelized activity. But here's my real concern,
though: we have API rules and promises associated with them, and this
violation could cause problems for third-party consumers of the RA
interface. I don't think we should cavalierly disregard the API rules in
this fashion, even if there are performance benefits to be had. And I
suspect that we could still be getting most of these performance benefits
anyway without violating the editor rules.

But what do others think? Where do we go from here?

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand