[ 
https://issues.apache.org/jira/browse/FLINK-5559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15834125#comment-15834125
 ] 

ASF GitHub Bot commented on FLINK-5559:
---------------------------------------

Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3172#discussion_r97279222
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/query/netty/message/KvStateRequestSerializer.java
 ---
    @@ -377,22 +376,24 @@ public static Throwable 
deserializeServerFailure(ByteBuf buf) throws IOException
                                0,
                                serializedKeyAndNamespace.length);
     
    -           K key = keySerializer.deserialize(dis);
    -           byte magicNumber = dis.readByte();
    -           if (magicNumber != 42) {
    -                   throw new IllegalArgumentException("Unexpected magic 
number " + magicNumber +
    -                                   ". This indicates a mismatch in the key 
serializers used by the " +
    -                                   "KvState instance and this access.");
    -           }
    -           N namespace = namespaceSerializer.deserialize(dis);
    +           try {
    +                   K key = keySerializer.deserialize(dis);
    +                   byte magicNumber = dis.readByte();
    +                   if (magicNumber != 42) {
    +                           throw new IOException("Unexpected magic number 
" + magicNumber + ".");
    +                   }
    +                   N namespace = namespaceSerializer.deserialize(dis);
     
    -           if (dis.available() > 0) {
    -                   throw new IllegalArgumentException("Unconsumed bytes in 
the serialized key " +
    -                                   "and namespace. This indicates a 
mismatch in the key/namespace " +
    -                                   "serializers used by the KvState 
instance and this access.");
    -           }
    +                   if (dis.available() > 0) {
    +                           throw new IOException("Unconsumed bytes in the 
serialized key and namespace.");
    +                   }
     
    -           return new Tuple2<>(key, namespace);
    +                   return new Tuple2<>(key, namespace);
    +           } catch (IOException e) {
    --- End diff --
    
    it may, but there are other `IOException`s in deeper code, e.g. in 
`dis.readByte();` and I did not want any special code separating these from our 
`IOException`s and considered this the least obtrusive variant


> queryable state: KvStateRequestSerializer#deserializeKeyAndNamespace() throws 
> an IOException without own failure message if deserialisation fails
> -------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: FLINK-5559
>                 URL: https://issues.apache.org/jira/browse/FLINK-5559
>             Project: Flink
>          Issue Type: Improvement
>          Components: Queryable State
>    Affects Versions: 1.2.0
>            Reporter: Nico Kruber
>            Assignee: Nico Kruber
>            Priority: Minor
>             Fix For: 1.2.0, 1.3.0
>
>
> KvStateRequestSerializer#deserializeKeyAndNamespace() throws an IOException, 
> e.g. EOFException, if the deserialisation fails, e.g. there are not enough 
> available bytes.
> In these cases, it should instead also throw an IllegalArgumentException with 
> a message containing "This indicates a mismatch in the key/namespace 
> serializers used by the KvState instance and this access." as the other error 
> cases.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to