DRAFT: Demonstrating fallible index traits

[bts]

Hi everyone!

I'm not planning to submit this PR as is. I'm posting this to get some thoughts on this line of work, which converts most methods on Index, MutableIndex, ReadonlyIndex, and ChangeIdIndex to have fallible (i.e. Result) return values. The motivation here is supporting distributed implementations of these index traits which might perform I/O during most operations. This way, implementers would not be forced to keep entire indices in memory. The Backend, OpStore, and OpHeadsStore storage traits already support distributed/network implementations, so these changes bring the *Index traits in line with those three.

If you look at the changes, note that I purposefully did not include a similar conversion of ReadonlyIndex::start_modification() -- that's because the consequences are quite a bit more involved than the other methods, due to causing WorkspaceCommandHelper::start_transaction() to become fallible. From what I can tell, I think most distributed/network implementations of the storage traits would probably be able to get by without making this one fallible by using an interior mutability pattern. (I could go into more detail if that's helpful).

FWIW:

• if we decide to move forward with something like these changes, I'd submit only a few related changes at a time to keep the reviews manageable.
• in a few places, rather than introduce more sweeping changes to unify errors (e.g. where we now produce IndexError in addition to BackendError), I erred toward sprinkling in a few more instances of the existing // TODO: indexing error shouldn't be a "BackendError"

Thanks!
Brian

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[PhilipMetzger]

All in all this looks great. I do think there are some minor codestyle issues and some helpers missing for all the tests which unwrap by default now.

I also think Yuya should take a look at it since he's the expert of that part of the codebase.

[bts]

Thank you @PhilipMetzger! I'll fix up these items in their respective changes.

[PhilipMetzger]

With some last minor nits this looks good enough to split into 2/3 smaller PRs because this is large and it reduces reviewer time. I'll also let Yuya be the judge of your more functional rewrites of some filter(...).map(...) calls into filter_map.

[bts]

@yuja @martinvonz @PhilipMetzger Thanks for your help everyone! I believe I've applied all of the suggestions above. I think we can probably start shifting to PRs at this point -- going to close out this draft. I put the first few changes are in #7799.