gesterzhou commented on a change in pull request #6561:
URL: https://github.com/apache/geode/pull/6561#discussion_r657666039
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/QueryOp.java
##########
@@ -120,7 +122,11 @@ protected Object processResponse(Message msg) throws
Exception {
queryResult = resultPart.getObject();
} catch (Exception e) {
String s = "While deserializing " + getOpName() + " result";
- exceptionRef[0] = new SerializationException(s, e);
+ if (e instanceof PdxSerializationException) {
+ exceptionRef[0] = new IOException(s, e);
Review comment:
There're 4 questions. I will answer them one by one:
(1) If there's a real PdxSerialization corruption happened at server side
caused by application (it did not happen so far though. Customer has verified
all their server pdx types and they kept verifying from time to time). It's
user's responsibility to fix their pdx serialzation issue in their application
code. Our responsibility is to tell the user that their pdx type is wrong.
That's why I add a dunit test to prove that in such a case, the client will
fail after retry. My retry logic won't retry forever. It will try all servers
then return with exception. You concern if we should fail earlier? I think we
are fine to take a few more seconds to double check, because customer has to
fix their pdx type in such a case, that will take hours or even days.
(2) Customer is following the document you mentioned to use pdx type in
correct way. That's why the client query correctly even before and after the
occasional exception. If they are using the wrong way, they will encounter
problem everyday.
The application is querying the pdx object's byte array from server, then
deserialized it at client. This is our default behavior. We offer another
non-default usage is: set --read-serialized=true, then the client will not
deserialize the byte array and wrap it as a PdxInstance object. It will delay
the deserialization into their application logic field-by-field. But that's not
our default setting.
(3) If application is using standard Java serialization, then it will not
fail with PdxSerializationException. It will not fail with
ObjectStreamException either. It will fail with SerialzationException, which is
handled in "else" (see my diff). I purposely did not change
SerialzationException into IOException. The reason is as customer support
ticket, we only fix what the issue is. If someday we encounter
SerialzationException caused by socket corruption, we can fix it later, but not
right now. If we concern about SerialzationException at client side should be
retried, we can log a new geode ticket.
(4) I suspected some socket corruption happened, which caused the Message
packed wrong byte array. From log, every reproduce of the
PdxSerializationException failed in different byte.
But since it's very hard to reproduce this issue, we have no clue where the
trigger is. Maybe in readHeaderAndBody() or readPayloadFields(). Maybe not.
Again, if we think these codes have potential issues, we can log a new geode
ticket.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
