[tahoe-lafs-trac-stream] [tahoe-lafs] #1382: immutable peer selection refactoring and enhancements

[tahoe-lafs]

#1382: immutable peer selection refactoring and enhancements
-------------------------+-------------------------------------------------
     Reporter:  kevan    |      Owner:  markberger
         Type:           |     Status:  new
  enhancement            |  Milestone:  1.11.0
     Priority:  major    |    Version:  1.8.2
    Component:  code-    |   Keywords:  review-needed servers-of-happiness
  peerselection          |  blocks-release
   Resolution:           |
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Comment (by zooko):

 Okay, so about the "try the first 2*{{{N}}} servers" part
 ([https://github.com/markberger/tahoe-
 lafs/blob/fa6f3cfb1e258e4427f35a7aada05c7ad2c9dd13/src/allmydata/immutable/upload.py#L340
 line 340]) that I mentioned in comment:55.

 There are two limitations of this. The first one relatively simple to
 solve and I'd like to do it in this branch. The second one is more
 invasive and I'd like to open a separate ticket for it.

 The first one is that all 2*N of the first servers might be in read-only
 mode (and not already have shares of this file). Then the upload would
 fail. This is actually fixable without waiting for further network
 results, because whether the servers were already announced to be in read-
 only mode is already known (through the introduction protocol) by the
 client, so we could for example do something like the following. Replace:
 {{{
     candidate_servers = all_servers[:2*total_shares]
     for server in candidate_servers:
         self.peer_selector.add_peer(server.get_serverid())
     writeable_servers = [server for server in candidate_servers
                         if _get_maxsize(server) >= allocated_size]
     readonly_servers = set(candidate_servers) - set(writeable_servers)
     for server in readonly_servers:
         self.peer_selector.mark_full_peer(server.get_serverid())
 }}}
 with:
 {{{
     num_of_writeable_servers = 0
     for server in all_servers:
         self.peer_selector.add_peer(server.get_serverid())
         if _get_maxsize(server) >= allocated_size:
             num_of_writeable_servers += 1
             if num_of_writeable_servers >= 2*total_shares:
                 break
         else:
             self.peer_selector.mark_full_peer(server.get_serverid())
 }}}
 I guess we ought to write a unit test of this! Demonstrating a situation
 where the first 2*N servers are all read-only so the upload fails with the
 old code but succeeds with the new code.

 The second limitation is the "deeper" one. Even if we pick 2*N writeable
 servers at this stage, there's no guarantee that any of those 2*N
 writeable servers will actually accept our subsequent writes. It could be
 that those 2*N of them are all full or read-only-mode now (even though
 they weren't back when the introduction protocol ran and informed our
 client about those servers), or they might all fail at this moment, or
 something. The current behavior is to give up on the upload if this
 happens. It would almost always be better to instead go ahead and ask the
 *next* few servers if they would hold shares.

 I'm going to open a separate ticket for that…

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1382#comment:56>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage