Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.4 tentacle break proposal #305

Closed
driftluo opened this issue Jan 27, 2021 · 5 comments
Closed

0.4 tentacle break proposal #305

driftluo opened this issue Jan 27, 2021 · 5 comments
Labels
breaking change The feature breaks consensus, database, message schema or RPC interface.

Comments

@driftluo
Copy link
Collaborator

driftluo commented Jan 27, 2021

tokio/bytes upgrade to 1.0

PR #293

TargetSession/TargetProtocol change

Currently, these two enumerated types under multi will force users to save all known session/protocol ids and specify them accurately, otherwise, they cannot be selectively broadcast, but this is not a good decision.

There are two proposals(mutually exclusive):

  1. Add a tag like Exclude(HashSet<ID>), it can remove some known options, but you don’t need to know all the options(can manually control the sending sequence)
  2. Change Multi(Vec<SessionId>) to Filter(Box<Fn(ID) -> bool + send + ‘static>), also achieved the purpose of flexibility(can't manually control the sending sequence )

Remove the deprecated event output mode

PR #293

Remove flutbuffer codec

PR #293

Extract core crate

Separate runtime encapsulation and transport layer into one library, and tentacle as a high-level encapsulation framework

Based on this design, it may be possible to implement a tentacle client to cover #205

Low priority, will not break API, can be done after release 0.4

@driftluo driftluo changed the title 0.4 tentacle break proposol 0.4 tentacle break proposal Feb 1, 2021
@doitian
Copy link
Member

doitian commented Feb 1, 2021

It seems that you also cannot specify the sending order using Exclude(HashSet<ID>).

@driftluo
Copy link
Collaborator Author

driftluo commented Feb 1, 2021

It seems that you also cannot specify the sending order using Exclude(HashSet<ID>).

But you can just use Multi(Vec<ID>) to specify the order

@doitian
Copy link
Member

doitian commented Feb 2, 2021

I prefer leaving Multi(Vec<SessionId>) and adding Filter(Box<Fn(ID) -> bool + send + ‘static>).

@driftluo
Copy link
Collaborator Author

std has introduced the Stream trait, GAT, and existence types are also advancing. In the long run, async strait must become the first sequence of abstractions, although, at present, there is still a long way to go

The trait design of the current library actually follows the design of the 0.1 eras, when await did not appear, but, at present, it is already usable. We should consider switching the current trait to async trait so that users can better use the asynchronous environment, although, the current macro implementation has a certain performance loss

But this change will be a big break change for users, and some dependencies need to be added, It is expected that at least the tokio macro and async-trait crates are to be added, and at the same time, some of the code needs to be rewritten. At the same time, whether poll fn can be retained is an unverified function, which needs to be considered after verification

If there is a lock dependency in the user's implementation, perhaps the user needs to consider the use of locks.

@driftluo
Copy link
Collaborator Author

Regarding async-trait, during the upgrade process, it is inevitable that part of the code needs to be rewritten. Based on stability and functional requirements, the rewritten part is only related to the user interface and has nothing to do with other modules.

Specifically, Service and protocol_handle_stream will be rewritten.

Instead of implementing the Stream trait, Service provides a startup function with the signature:

async fn run(&mut self) -> Option<()>

Used to adapt tokio::select! macro, is return None, no longer poll it.

ServiceHandle will change to:

#[async_trait]
pub trait ServiceHandle {
    async fn handle_error(&mut self, _control: &mut ServiceContext, _error: ServiceError) {}
    async fn handle_event(&mut self, _control: &mut ServiceContext, _event: ServiceEvent) {}
}

ServiceProtocol will change to:

#[async_trait]
pub trait ServiceProtocol {
    async fn init(&mut self, context: &mut ProtocolContext);
    async fn connected(&mut self, _context: ProtocolContextMutRef, _version: &str) {}
    async fn disconnected(&mut self, _context: ProtocolContextMutRef) {}
    async fn received(&mut self, _context: ProtocolContextMutRef, _data: bytes::Bytes) {}
    async fn notify(&mut self, _context: &mut ProtocolContext, _token: u64) {}
    #[inline]
    async fn poll(
        self: &mut Self,
        _cx: &mut Context,
        _context: &mut ProtocolContext,
    ) -> Option<()> {
        None
    }
}

SessionProtocol is similar to it. poll method is reserved to preserve the original custom wake-up function.

The Control inside the Context will be converted from a sync method to an async method, supporting await, but retaining the function of converting the sync structure

BlockingFlag will be removed, which means that tentacle no longer uses the block_in_place method by default, and users need to decide whether to use it or not.

The use of the above traits will depend on tokio::select!, which means dependence will include it.

We all know that await is the yield point of async code, which means that in distribution mode if a distribution point is pending, it will not perform subsequent distribution work until it is awakened again. Therefore, the poll mode of the session module will not be rewritten to await mode, just to detect and skip this situation, the link detection from service to the session has been moved to the session to protocol stream, so the rewriting does not affect its own function

@driftluo driftluo added the breaking change The feature breaks consensus, database, message schema or RPC interface. label Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The feature breaks consensus, database, message schema or RPC interface.
Projects
None yet
Development

No branches or pull requests

2 participants