[Runtime Epoch Split] (3/n) Add ability to get Arc<EpochManagerAdapter> out of an &RuntimeWithEpochManagerAdapter.

[robin-near]

As a reminder of the overall goal, we want to split RuntimeWithEpochManagerAdapter, so that any code that needs the runtime will use an Arc<RuntimeAdapter>, and any code that uses the epoch manager will use the EpochManagerHandle.

We're doing this refactoring bottom-up, i.e. propagating RuntimeWithEpochManagerAdapter around at the top-level, but making some lower-level components use Arc<RuntimeAdapter> and/or EpochManagerHandle.

That means we need to be able to obtain an Arc<RuntimeAdapter> and an EpochManagerHandle from a RuntimeWithEpochManagerAdapter. However, this is not trivial at all:

1. KeyValueRuntime, the implementation of RuntimeWithEpochManagerAdapter for testing, does not contain an EpochManager at all, so it's not possible to extract an EpochManagerHandle from it (which is essentially an arc mutex of EpochManager). That means instead of using EpochManagerHandle, we need to use Arc<EpochManagerAdapter> in the meantime.
2. Extracting an Arc<EpochManagerAdapter> from a Arc<RuntimeWithEpochManagerAdapter> is not trivial. Even though RuntimeWithEpochManagerAdapter is a trait that extends EpochManagerAdapter, trait upcast is not allowed by Rust in general. So we need to resort to a workaround.

This PR implements the workaround for (2):

pub trait RuntimeWithEpochManagerAdapter: RuntimeAdapter + EpochManagerAdapter {
    fn epoch_manager_adapter(&self) -> &dyn EpochManagerAdapter;
    fn epoch_manager_adapter_arc(&self) -> Arc<dyn EpochManagerAdapter>;
}

How to implement epoch_manager_adapter_arc from a &self? We'll use a self-referencing Weak<Self> that implementations must keep as a field. This can be done using Arc::new_cyclic in the constructor. As a result, we also enforce that all runtime implementations be used always with an Arc (or else the Weak would go out of scope). This isn't a problem, because we already require the use of Arc<RuntimeWithEpochManagerAdapter> everywhere. To implement epoch_manager_adpater_arc, we upgrade the Weak<Self> into an Arc<Self> which always succeeds because we always use Arc (there's no way to construct without returning an Arc), and then we return the Arc<Self> which succeeds because Self implements EpochManagerAdapter. Self here refers to KeyValueRuntime and NightshadeRuntime.

This is an interim strategy; when all the refactoring and test migrations are complete, KeyValueRuntime would be deleted and we would not have EpochManagerAdapter or RuntimeWithEpochManagerAdapter.

[robin-near]

This is one of the files with changes worth reviewing.

[robin-near]

This is one of the files with changes worth reviewing.

[robin-near]

This is one of the files with changes worth reviewing.

The change here is due to Arc no longer being able to support a &mut self function. So instead we pass in the tracks_all_shards parameter via a factory function.

[robin-near]

This is one of the files with changes worth reviewing.

[jakmeier]

Thanks for the awesome PR description! It really helps to understand the intention of the change before looking at the code and that makes the review itself much easier & faster.

I'm happy to merge this as is. Although, as I mentioned in a comment, I don't think this is the ideal way of doing it. But since you plan to remove this code again soon anyway, I don't see the value in making this part perfect right now. Better to keep moving in many small steps towards the final state we want the code to be in.

[jakmeier]

As a intermediate step to get from A to B, I can tolerate this. But I think you made your life harder than it needed to be. :)

In Rust, trait inheritance really isn't inheritance as one expects from OOP and all attempts to use in such way will result in pain. I would have probably chosen a composition strategy, something along the lines of:

struct RuntimeWithEpochManagerAdapter {
    runtime: Arc<dyn RuntimeAdapter>,
    epoch_manager: Arc<dyn EpochManagerAdapter>,
}

Then you can use this struct in places the trait RuntimeWithEpochManagerAdapter was used before.

trait RuntimeWithEpochManagerAdapter: RuntimeAdapter + EpochManagerAdapter {} may look like it gives you the same. But it doesn't. It is only a short-hand for the type bound RuntimeAdapter + EpochManagerAdapter, which is still checked on each type separately at compile-time and doesn't have a way to cast at runtime.

[robin-near]

You are totally right. This honestly was my oversight, because I used that exact same pattern you suggested for the TestLoop, but not here - and I used it exactly to solve this problem!

Well nonetheless, thinking about it now, there is a non-trivial amount of boilerplate required with this solution, because any existing user of RuntimeWithEpochManagerAdapter (which should be stripped of the Arc<dyn >) would need to either call functions via .runtime or .epoch_manager (which would be a simple but time-consuming refactoring), or we would need to define forwarding functions on RuntimeWithEpochManagerAdapter, one for each function of RuntimeAdapter and EpochManagerAdapter.

Oh well, I suppose given that we're already here, it perhaps is better to use the time to getting rid of this whole thing faster.