Shutdown request handler before network server in ambry server

[justinlin-linkedin]

No description provided.

[codecov-io]

Codecov Report

Merging #1206 into master will not change coverage.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master    #1206   +/-   ##
=========================================
  Coverage     69.43%   69.43%           
- Complexity     5510     5511    +1     
=========================================
  Files           432      432           
  Lines         33844    33844           
  Branches       4312     4312           
=========================================
  Hits          23501    23501           
- Misses         9149     9150    +1     
+ Partials       1194     1193    -1

Impacted Files	Coverage Δ	Complexity Δ
...main/java/com.github.ambry.server/AmbryServer.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...java/com.github.ambry.store/CompactionManager.java	87.33% <0%> (-2.67%)	19% <0%> (ø)
...github.ambry.rest/AsyncRequestResponseHandler.java	87.94% <0%> (-0.66%)	23% <0%> (ø)
...java/com.github.ambry.network/SSLTransmission.java	69.1% <0%> (-0.64%)	67% <0%> (-2%)
...va/com.github.ambry.replication/ReplicaThread.java	74.62% <0%> (-0.19%)	66% <0%> (-1%)
...src/main/java/com.github.ambry.commons/BlobId.java	93.54% <0%> (+0.35%)	72% <0%> (+1%)	⬆️
...in/java/com.github.ambry.network/SocketServer.java	84.01% <0%> (+0.4%)	15% <0%> (ø)	⬇️
...in/java/com.github.ambry.store/BlobStoreStats.java	71.54% <0%> (+0.61%)	104% <0%> (+1%)	⬆️
...b.ambry.network/BlockingChannelConnectionPool.java	70.42% <0%> (+0.93%)	8% <0%> (+1%)	⬆️
.../main/java/com.github.ambry.router/DecryptJob.java	97.22% <0%> (+5.55%)	5% <0%> (+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 80c32e7...10d54c4. Read the comment docs.

[cgtz]

I wonder what will happen for the requests that are currently being handled by requestHandlerPool if networkServer is shutdown? I think from the client side, they should see the disconnect quickly, but we have to double check that the RequestHandlers won't error out if the network components are shut down (my guess is no since they communicate via queues, but still worth checking)

[justinlin-linkedin]

Some experiments on the server side are definitely needed.

All the communication between network server and the request handler is through a concurrent blocking queue, so you are right about that request handlers won't error out. After this change, what should be happening is that, when we shutdown the request handler, it will handle all the pending requests in the blocking queue and enqueue corresponding responses to yet another blocking queue. But all the requests received during that will not be handled at all (request handlers are shut down). And there is no guarantee that the responses will be sent out either. My thought on this change is that, it's probably going to gracefully handle some pending requests but not all.

Even w/o this change, the client should be notified fairly soon since the the server might just RST the connection. Either way, clients need to be aware of the sudden termination of the connections.