gianm commented on issue #7256: fix SequenceMetadata deserialization
URL: https://github.com/apache/incubator-druid/pull/7256#issuecomment-472444899
 
 
   > I don't love this fix, am totally open to other ideas.
   
   What don't you love about it?
   
   > This also means SequenceMetadata has been pulled out of 
SeekableStreamIndexTaskRunner and given the same generic parameters of 
PartitionIdType and SequenceOffsetType. This was sort of ugly because 
SequenceMetadata was calling methods on it's parent 
SeekableStreamIndexTaskRunner, so those methods now take a runner as an 
argument.
   
   I haven't read the patch yet (will soon), but, I don't necessarily think 
that this kind of change is ugly! The new structure might even be better. The 
SequenceMetadata in master looks at first like a simple state class, but it 
actually has methods that modify the runner it came from, which isn't intuitive 
to me. Making it a non-inner class and explicitly passing in the runner could 
make that clearer.
   
   > Also fixed is an issue where a resumed task that was at the end offset 
would not correctly end the task, resulting in what was afaict a task stuck in 
it's read loop forever.
   
   Is this related to #7252 or a separate thing you fixed opportunistically?

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to