THRIFT-6107: Preserve NonblockingServer reply order

[kpumuk]

Ruby's NonblockingServer can dispatch multiple requests from one connection to different workers. When a later handler finishes first, it can write its reply before an earlier request’s reply, which breaks clients that intentionally pipeline requests on that connection.

This change gives each connection an ordered response queue. A completed reply waits until all earlier request sequences have been published; oneway calls, handler failures, write failures, connection removal, and shutdown advance or discard the affected queue safely. Requests on separate connections continue to run independently.

• Did you create an Apache Jira ticket? THRIFT-6107
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[Copilot]

Pull request overview

This PR addresses a correctness issue in the Ruby Thrift::NonblockingServer where replies on a single connection could be written out of order when requests are dispatched to different worker threads and later handlers finish first. It introduces a per-connection ordered response queue so replies are only published in contiguous request sequence order, while keeping independent connections concurrent.

Changes:

• Add a per-connection IOManager::ResponseQueue that reserves request sequence IDs, bounds in-flight work, and publishes completed responses in order.
• Update IOManager/Worker flow so workers write responses into an in-memory buffer and the response queue serializes writes to the underlying connection transport.
• Expand Ruby specs to validate ordered reply publication and cover edge cases (oneway handling, failures, disconnect/cleanup, capacity/backpressure).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
lib/rb/spec/nonblocking_server_spec.rb	Adds end-to-end and unit-level specs validating in-order replies per connection and response-queue edge cases.
lib/rb/lib/thrift/server/nonblocking_server.rb	Implements per-connection response ordering, buffering, and capacity signaling within NonblockingServer::IOManager.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Jens-G]

Code review

Found 1 issue:

1. ResponseQueue#complete holds @mutex while performing a blocking socket write (write_response calls @transport.write/flush inside the lock). The same mutex is required by reserve, called only from the single IOManager thread's read/dispatch loop for every connection — so one client that stops draining its receive buffer while a reply write is in flight can stall dispatch for all other connections too, for up to client_timeout (default 5s; unbounded if client_timeout: nil). This contradicts the PR's stated goal that "requests on separate connections continue to run independently."

thrift/lib/rb/lib/thrift/server/nonblocking_server.rb

Lines 438 to 461 in d8da61a

	def complete(sequence, response)
	error = nil
	capacity_available = false
	@mutex.synchronize do
	return if @closed || sequence < @next_to_publish
	was_full = full?
	begin
	publish_response(sequence, response)
	capacity_available = was_full && accepting_without_lock?
	rescue IOError, SystemCallError, TransportException => e
	@logger.debug "#{self} could not write response: #{e.inspect}"
	close_without_lock
	error = e
	end
	end
	if error
	@on_error&.call(@transport, error)
	elsif capacity_available
	@on_capacity&.call(@transport)
	end
	end

thrift/lib/rb/lib/thrift/server/nonblocking_server.rb

Lines 497 to 504 in d8da61a

	def write_response(response)
	unless response.nil? || response.empty?
	@transport.write(response)
	@transport.flush
	end
	@next_to_publish += 1
	@in_flight -= 1
	end

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}