flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-6018) Properly initialise StateDescriptor in AbstractStateBackend.getPartitionedState()
Date Wed, 15 Mar 2017 19:03:41 GMT

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

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

Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/3534
  
    This change may alter the format of savepoints, because it now forwards type-registrations
to the Kryo serializer, which it did not do before.
    
    Since we announced savepoint compatibility, we need to understand when this would happen.
Is it something that could not really happen before (because whenever the method was called,
the serializer was properly initialized outside), or did this actually occur in some cases?
    
    @sunjincheng121 Can you share how you stumbled across this bug? Was it a code cleanup,
or a bug you encountered while running Flink?


> Properly initialise StateDescriptor in AbstractStateBackend.getPartitionedState()
> ---------------------------------------------------------------------------------
>
>                 Key: FLINK-6018
>                 URL: https://issues.apache.org/jira/browse/FLINK-6018
>             Project: Flink
>          Issue Type: Improvement
>          Components: DataStream API, State Backends, Checkpointing
>            Reporter: sunjincheng
>            Assignee: sunjincheng
>             Fix For: 1.3.0
>
>
> The code snippet currently in the `AbstractKeyedStateBackend # getPartitionedState` method,
as follows:
> {code}
> line 352: // TODO: This is wrong, it should throw an exception that the initialization
has not properly happened
> line 353: if (!stateDescriptor.isSerializerInitialized()) {
> line 354:        stateDescriptor.initializeSerializerUnlessSet(new ExecutionConfig());
> line 354 }
> {code}
> Method `isSerializerInitialized`:
> {code}
> public boolean isSerializerInitialized() {
> 		return serializer != null;
> 	}
> {code}
> Method `initializeSerializerUnlessSet`:
> {code}
> public void initializeSerializerUnlessSet(ExecutionConfig executionConfig) {
> 		if (serializer == null) { 
> 			if (typeInfo != null) {
> 				serializer = typeInfo.createSerializer(executionConfig);
> 			} else {
> 				throw new IllegalStateException(
> 						"Cannot initialize serializer after TypeInformation was dropped during serialization");
> 			}
> 		}
> 	}
> {code}
> that is, in the `initializeSerializerUnlessSet` method, The `serializer` has been checked
by `serializer == null`.So I hope this code has a little improvement to the following:
> approach 1: 
> According to the `TODO` information  we throw an exception
> {code}
> if (!stateDescriptor.isSerializerInitialized()) {
> 			throw new IllegalStateException("The serializer of the descriptor has not been initialized!");

> }
> {code}
> approach 2:
> Try to initialize and remove `if (!stateDescriptor.isSerializerInitialized()) {` logic.
> {code}
> stateDescriptor.initializeSerializerUnlessSet(new ExecutionConfig());
> {code}
> Meanwhile, If we use the approach 2, I suggest that `AbstractKeyedStateBackend` add a
`private final ExecutionConfig executionConfig` property. then we can change the code like
this:
> {code}
> stateDescriptor.initializeSerializerUnlessSet(executionConfig);
> {code}
> Are the above suggestions reasonable for you? 
> Welcome anybody's feedback and corrections.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message