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

[Release] Checklist #290

Open
6 of 7 tasks
OlegDokuka opened this issue Feb 5, 2020 · 7 comments
Open
6 of 7 tasks

[Release] Checklist #290

OlegDokuka opened this issue Feb 5, 2020 · 7 comments

Comments

@OlegDokuka
Copy link
Member

OlegDokuka commented Feb 5, 2020

Release 1.0 Checklist

  • Core Specification
  • Composite Metadata Extension
  • Routing Metadata Extension
  • Tracing Metadata Extension
  • Data MimeType Extension
  • Auth Metadata extension
  • Removing Leasing from spec Move leasing out of core spec (follow up on [Important] Updates to Leasing Specification #273)
@mostroverkhov
Copy link
Member

Why protocol intended for efficient transmission of byte streams would be concerned about data layout? Would those extensions make more sense for RSocket-RPC than for RSocket itself? Without requests leasing there is no way to limit requests concurrency - queues will fill up thus making reactive streams semantics of single stream pointless - I have custom implementation of that based on https://github.com/Netflix/concurrency-limits.

@mostroverkhov
Copy link
Member

mostroverkhov commented Feb 5, 2020

With proposed feature set RSocket looks like http2 streams twin brother - why have another protocol at all? Wondering what were the parties involved when this list was defined.

@OlegDokuka
Copy link
Member Author

OlegDokuka commented Feb 6, 2020

I totally share your concerns about removing Leasing spec from the core spec.

The plan is to make a release and in order to make it earlier, we have to remove the current state of leasing since it is non-finalized. It is going to be returned after revising. Please see #273 for the reasoning on why it's removing from the spec is happening right now.

Any ideas on how we can move Leasing spec to the extensions are more than appreciated.

@mostroverkhov
Copy link
Member

mostroverkhov commented Feb 6, 2020

Thanks for sharing your vision of the specification - but (without belittling your expertise on a matter) the original one was designed as joint effort of Netflix, Facebook and Todd Montgomery ( person behind Aeron) in an open discussion fashion, and there were no issues with current specification so far. And I am sure Robert can explain message count with Reactive Streams vs byte count leasing impedance in #273, the question is still open:

  • why are there data layout definitions in specification for byte streams protocol? They are just glorified headers definitions - why cant I just use HPACK packed headers? Why spec every header if there are well-known (and custom) metadata types?
  • who endowed you with changing important parts of the specification in such assertive tone?

@OlegDokuka
Copy link
Member Author

OlegDokuka commented Feb 6, 2020

Just FYI, it is not mine, right now I'm responsible for the process but this is a collective decision.

All the stakeholders from Facebook, Netflix and Pivotal are aware of what is going on. You may be sure, as before it is a joint effort.

why are there data layout definitions in the specification for byte streams protocol?

I'm not quite sure what are you talking about. The mentioned checklist includes version 1.0 for the core spec and 0.0 for extension spec. As it was before, the extension specifications are living independently and do not impact anyhow the core spec.

bytestreams protocol

It is working like this as before, the extensions are optional and work in case of Composite Metadata Mimetype

headers definitions - why can't I just use HPACK packed headers

As before, the header metadata format can be defined at the connection setup phase so there are no constraints on metadata layout

who endowed you with changing important parts of the specification in such assertive tone?

If you have any concerns, please, feel free to get in touch with any of Reactive Foundation members

@mostroverkhov
Copy link
Member

mostroverkhov commented Feb 7, 2020

collective decision

This means specification changes and road map switched from public discussion through issues on this project - to private discussion and decision making by small circle of companies listed on https://reactive.foundation/members/ ? In particular, group decided privately that feature set outlined at http://rsocket.io/docs/Implementations is superseded by above list? Foundation governance is significant change, and should have been reflected on rsocket.io.

get in touch with any of Reactive Foundation members

I'd like to hear members opinion on

  • what are the options for rate limiting RSockets once leasing is removed; whether demand limiting with reactive streams is useful if # of requests is unbounded
  • presence of routing and tracing extensions in specification mainly concerned with byte streams (there are no mentions of RPC in spec);
  • presence of authentication extension - with Http this is one of many headers, chances are high http2 will be used as transport for connections outside of datacenter => more headers should be mapped to RSocket stream - may be prefer set of key-value pairs instead? (still only useful in RPC context)

@OlegDokuka
Copy link
Member Author

OlegDokuka commented Feb 8, 2020

@mostroverkhov I'm not going to go in any discussion here anymore since this issue alive here to track the TODO list on the final release.

RSocket Extensions are optional and the concrete of extensions was defined at the very beginning of the specification development. Right now, it is expanding due to users' requirements.

The spec development process is going on as before. Reactive Foundation is an expert group that leads that project, but as before, community input is equally valuable.

Removing / Rethinking of leasing will be going as a PR process, so you may share your thoughts once the relevant issue / PR is going to appear.

If you have any questions/suggestions, there is an issue process, so feel free to open one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants