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

Migrate to ktor 2.0.0 eap and new native memory model #208

Merged
merged 1 commit into from
Feb 26, 2022

Conversation

olme04
Copy link
Contributor

@olme04 olme04 commented Feb 7, 2022

Main reason for using ktor 2.0.0 EAP is support of coroutines 1.6 (non native-mt) and new native memory model. On current moment native-mt version of coroutines has some bugs, that appear in random places, and have no workarounds. In non native-mt there is no such bugs, and as anywhere, after release of ktor 2.0.0 we will need to migrate to new memory model, as ktor 2.0.0 will support only it, such change is both pro-active, and will improve developer experience.

Changes:

  • split ktor transport implementation (required for ktor 2.0.0)
    • changed from: transport-ktor, transport-ktor-client/server to transport-ktor-tcp, transport-ktor-websocket-client/server + intermediate modules transport-ktor and transport-ktor-websocket
    • starting from ktor 2.0.0 ktor-network module isn't published for JS - before it was published but had not implemented errors instead of implementations
  • use 1.6.20 eap kotlin version
    • needed for ktor and new memory model
    • 1.6.20 will be in M1 in coming days, and release of it will be in near month
    • no release of rsocket-kotlin is expected in coming month
  • use 2.0.0 eap ktor version
    • main change needed is support of new memory model
    • minimal code update to be compatible with ktor 2.0.0
  • use coroutines 1.6.0
    • no specific features except native new memory model is used
    • need to investigate if it's feasible to use new multiplatform coroutines-test module
  • minor dependencies versions update
  • remove coroutines version replacement strategy, as it's not needed any more, as we don't use native-mt version anymore
  • enable new native memory model (this will also speed up our tests)

Note: in future PR's there will be another split of ktor transport dependencies:

  • make transport-ktor-websocket-server/client dependencies provide just plain transports similar to nodejs and local
  • create 2 new modules ktor-client/server with idea of deep ktor integration. For now it will be just client and server plugin, but in future, there will be possibility to integrate rsocket-kotlin into ktor-serialization or ContentNegotiation API, or even something else.

TODO: update readme with new modules later in one go after new transport API and ktor integration improvements will be implemented

* split ktor transport implementation (required for ktor 2.0.0)
* use 2.0.0 eap ktor version
* use 1.6.20 eap kotlin version
* use coroutines 1.6.0
* minor dependencies update
* minimal code update to be compatible with ktor 2.0.0
* remove coroutines version replacement strategy, as it's not needed any more
* enable new native memory model
@monsdroid
Copy link

Hi, would you mind to release this as an EAP or alpha?

@olme04
Copy link
Contributor Author

olme04 commented Feb 18, 2022

@monsdroid Hey, snapshot will be available just after merging this PR, but because of buggy tests, most likely snapshot will be available after merge of #210. All of this will be merged soon.
BTW, snapshots will be on Github Packages.

@whyoleg whyoleg merged commit 9b5c73e into rsocket:master Feb 26, 2022
@olme04 olme04 deleted the dev/1-ktor-2.0.0 branch February 26, 2022 06:03
@olme04
Copy link
Contributor Author

olme04 commented Mar 1, 2022

Hey @monsdroid, snapshots are published now, you can take a llok on them now.
Here is an example of using both ktor 2.0, rsocket-kotlin 0.15.0-SNAPSHOT and GithubPackages configuration olme04@6488dd9
Note: make sure to update all rsocket-ktor dependencies, as old snapshot versions will be used

@monsdroid
Copy link

Just wanted to say thanks. great work. after adjusting some imports everything works fine

@olme04
Copy link
Contributor Author

olme04 commented Mar 9, 2022

Im glad to hear it! thx!
Just FYI, coming snapshots will have breaking changes, and also for now you should use same ktor eap version, as used in rsocket-kotlin as ktor isn't backward compatible during EAP :)

@afTrolle
Copy link

Hello @olme04, First of great work with Rsocket.

I might have misunderstood something, I assumed that 0.15.0-SNAPSHOT contains this PRs changes

However the "io.rsocket.kotlin:rsocket-transport-ktor-server:0.15.0-SNAPSHOT" artifact is using code from before this PR.

When checking the RSocketSupport class still uses ApplicationFeature not the ApplicationPlugin

You can download the snapshot directly from the github artifacts page.

you can check the rsocket-transport-ktor-server-jvm-0.15.0-20220114.090008-1-sources.jar

@olme04
Copy link
Contributor Author

olme04 commented Mar 12, 2022

Hey, @afTrolle!
That's what I've mentioned in my previous comment and in PR description:

changed from: transport-ktor, transport-ktor-client/server to transport-ktor-tcp, transport-ktor-websocket-client/server + intermediate modules transport-ktor and transport-ktor-websocket

Note: make sure to update all rsocket-ktor dependencies, as old snapshot versions will be used

After this PR, there is no more rsocket-transport-ktor-server artifact published. You should use rsocket-transport-ktor-websocket-server.
But because previous snapshot artifacts has same 0.15.0-SNAPSHOT version (as snapshot is published on every push to master), but differs only in build id, both old and new artifacts are fetched by gradle.

You can also take a look here on the diff from 0.14 to 0.15 snapshot for sample:
olme04@6488dd9#diff-88768ccb1ab6ab9a9996fee2a7ba5e7ee44febec724d7cf17bf04d91f535b26f

Hope this helps!

@afTrolle
Copy link

Okay, thank you for the explanation.

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

Successfully merging this pull request may close these issues.

None yet

4 participants