L1nq0 opened a new pull request, #9076:
URL: https://github.com/apache/storm/pull/9076

   Closes #9074
   
   Upgrade note: a malformed message body no longer kills the receiving worker, 
including the IOException case. Previously a channel-level IOException closed 
the connection and lost every message still queued on it; now the one 
undecodable message is dropped and the connection, along with the rest of the 
batch, keeps going. Payloads that used to tear down a connection mid-stream 
will instead surface as deserializationFailures counts and per-message ERROR 
logs.
   
   What this changes
   
   DeserializingConnectionCallback.recv() now wraps each message's 
deserialization in a try/catch. When the failure's cause chain matches a known 
decode-time exception type, the message is dropped with an ERROR log (exception 
class and message, destination task id, payload length, and no payload bytes) 
and the loop continues with the next message in the batch. Anything outside 
that set is rethrown and keeps today's behavior: 
StormServerHandler.exceptionCaught, then Utils.handleUncaughtException, then 
worker exit. Errors are never caught.
   
   The tolerated set is the set of exception types a garbage or hostile byte 
stream can already produce during tuple decode, checked with the existing 
cause-chain helper Utils.exceptionCauseIsInstanceOf:
   
   IOException, KryoException, IllegalArgumentException, 
NegativeArraySizeException, ClassCastException, ArrayIndexOutOfBoundsException, 
BufferUnderflowException, NullPointerException, ClassNotFoundException
   
   Mapping to wire-level failure modes: truncated or negative length fields 
(ArrayIndexOutOfBoundsException, NegativeArraySizeException, 
BufferUnderflowException), unregistered classes under topology.kryo.register 
with registration required (IllegalArgumentException), classes present only on 
the sending side (ClassNotFoundException), bogus task ids inside a tuple 
(NullPointerException from task-info lookup), wrong runtime types 
(ClassCastException), and kryo's own decode failures (KryoException) plus 
underlying stream problems (IOException).
   
   I deliberately did not broaden StormServerHandler.ALLOWED_EXCEPTIONS. That 
whitelist is about transport-level channel errors and closing the connection; a 
payload that fails to decode is an application-level event, and handling it 
where the decode happens is what keeps the rest of the batch alive. The 
whitelist stays as the backstop for anything that still escapes.
   
   Failures are counted in a deserializationFailures counter exposed through 
the existing getValueAndReset() metrics contract: it stays null until a failure 
occurs when tuple-size metrics are disabled, matching how the adjacent 
serialization metrics behave.
   
   Tests
   
   9 cases in DeserializingConnectionCallbackTest, driving recv() through the 
real deserializer where possible: one tolerant drop per interesting exception 
type (verify log, counter, and batch continuation), an IOException mid-batch 
that skips exactly the bad message while a good message later in the same batch 
still lands, rethrow of a non-tolerated exception, counter null when nothing 
failed, and counter reset after getValueAndReset().
   
   Also verified on a live 3.0.1-SNAPSHOT cluster: replaying a 27-byte frame 
carrying an unregistered class against a worker port kills the worker on master 
(terminating server, then process exit and supervisor restart) and on this 
branch produces the log line "Failed to deserialize a message of 27 bytes 
destined for task 2, dropping it" with the worker surviving and the topology 
staying active.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to