#3779 closed task (fixed)

Ensure IStorageServer-using code doesn't have interactions that conflict with HTTP implementation

Reported by: itamarst Owned by: itamarst
Priority: normal Milestone: HTTP Storage Protocol
Component: unknown Version: n/a
Keywords: Cc:
Launchpad Bug:


On the assumption that we keep IStorageServer in place, and provide a HTTP implementation of the interface, current users of the interface need some updates.

In particular, some parts of the code make unwarranted assumptions about interaction patterns, e.g. that if command A is run before command B then command A will be started before command B. With an HTTP client, these commands will be sent in parallel, not necessarily over the same TCP connection as Foolscap does, so this assumption won't always be true.

For this particular example, the code would need to be modified so that B was only sent when a response is received from the A command, ensuring the required ordering.

Change History (5)

comment:1 Changed at 2021-08-25T16:55:49Z by itamarst

  • Summary changed from Ensure IStorageServer using code doesn't have interactions that conflict with HTTP implementation to Ensure IStorageServer-using code doesn't have interactions that conflict with HTTP implementation

comment:2 Changed at 2021-08-25T17:00:46Z by itamarst

To go the manual audit route, given a whole bunch of arbitrary fakes:

  1. For each method in IStorageServer and related objects (bucket reader/writer), grep for method definitions; eventually this should discover all fakes. E.g. allmydata.test.mutable.util.FakeStorageServer? is not annotated with the IStorageServer interface but implements it.
  2. For each fake, temporarily add logic to print tracebacks(?) to file.
  3. Temporarily add logic to print tracebacks to file for real code too.
  4. Run unit and integration tests.
  5. Uniquify file.
  6. Permanently add interface implementation annotation, so in the future we can just use grep.

At this point we should have locations of all calls into Foolscap storage protocol, and can audit it for bad assumptions like "order of calls matches order of delivery".

comment:3 Changed at 2021-09-01T14:58:10Z by itamarst

Could be expanded to removing use of callRemote/callRemoteOnly on the read/write buckets (and replace all use of callRemoteOnly while we're at with callRemote+log-on-error). or that could be a separate ticket.

Last edited at 2021-09-01T17:35:38Z by itamarst (previous) (diff)

comment:4 Changed at 2021-09-02T19:25:10Z by itamarst

Probably easiest to just fake callRemote in the IStorageServer adapter, but I did get rid of all callRemoteOnly.

comment:5 Changed at 2021-09-13T13:28:51Z by GitHub <noreply@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 2eafe41/trunk:

Merge pull request #1117 from tahoe-lafs/3779-istorageserver-with-fewer-assumptions

Reduce implementation-dependency of IStorageServer

Fixes ticket:3779

Note: See TracTickets for help on using tickets.