fix subscriptions by not removing the ak subscription when one of the…

[gklijs]

📝 Description

Prevents stopping ka when ending a subscription

🔗 Related Issues

#499 I hoped, but is not helped by it.

[codecov-io]

Codecov Report

Merging #510 into master will decrease coverage by 0.67%.
The diff coverage is 65.51%.

@@             Coverage Diff              @@
##             master     #510      +/-   ##
============================================
- Coverage      97.9%   97.23%   -0.68%     
  Complexity      335      335              
============================================
  Files           106      106              
  Lines          1290     1300      +10     
  Branches        207      210       +3     
============================================
+ Hits           1263     1264       +1     
- Misses            7       14       +7     
- Partials         20       22       +2

Impacted Files	Coverage Δ	Complexity Δ
...ing/execution/ApolloSubscriptionProtocolHandler.kt	82.08% <65.51%> (-12.65%)	12 <1> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b387839...7e0087a. Read the comment docs.

[smyrick]

@gklijs I tried checking out your performance benchmarks but the step up was taking a little bit longer than expected since I have not used clojure before

Is it possible that you can update the SubscriptionWebSocketHandler.handle method to this.

override fun handle(session: WebSocketSession): Mono<Void> {
        val response = session.receive()
            .map { it.payloadAsText }
            .flatMap { subscriptionHandler.handle(it, session) }
            .map { objectMapper.writeValueAsString(it) }
            .map { session.textMessage(it) }
            .doFinally { session.close() }

        return session.send(response)
    }

While the handler should close the session when receiving a GQL_CONNECTION_TERMINATE message, I just want to make sure that the session is in fact closing and not eating up the memory.

[smyrick]

On a side note, I tried using sending the data over as bytes instead of converting to a string to see if that improved performance, but I my quick change did not seem to work so I will need to look into that more

val response = session.receive()
    .map { it.payloadAsText }
    .flatMap { subscriptionHandler.handle(it, session) }
    .map { objectMapper.writeValueAsBytes(it) }
    .map { session.binaryMessage { dataBufferFactory -> dataBufferFactory.wrap(it) } }
    .doFinally { session.close() }

[gklijs]

I'll give it a try tomorrow. You indeed need leiningen (build tool for Clojure) and sassc installed. Now that I better know the problem I might try create a test in the project itself trough.
Binary data is handled differently as string values in a websocket so that might be reason it fails. I don't that's a good idea because clients might break because of it.

[gklijs]

The mentioned fix #510 (comment) didn't help. It's still stops after around 120 subscriptions being started and stopped.

[gklijs]

Finally kind of done updating the results (not all variants ran 10 times, would improve accuracy a bit, but not much) https://graphql.gklijs.tech/results/graphql-servers/transactions-per-second