carp84 commented on issue #9501: [FLINK-12697] [State Backends] Support on-disk state storage for spill-able heap backend URL: https://github.com/apache/flink/pull/9501#issuecomment-542879295 Thanks for the review @StephanEwen ! Checking the code I agree that there're similar goals between `ByteBufferUtils` and `MemorySegment`, will give it a more careful check about whether we could merge them. I admit it's my bad to directly borrow the classes ([UnsafeAccess](https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/util/UnsafeAccess.java) and [ByteBufferUtils](https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java)) from hbase and lack of knowledge about whether there's already similar functionalities in Flink. I checked `MemoryUtils` and didn't find enough util methods so goes the "easy" way. And the test was also following the hbase way ([TestByteBufferUtils](https://github.com/apache/hbase/blob/master/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestByteBufferUtils.java)), that covering all logic through `ByteBufferUtilsTest` instead of introducing an `UnsafeHelpTest`. Since the unsafe tool and unaligned logic in hbase has been used for a long time in different environments, I believe it should work fine, while I agree that it's better to have a unit test to cover the correctness checking. Will get back soon with updates after more thinking. PS. I'm still learning to switch between HBase and Flink code convention and hopefully this is not causing big problem, and my apology if already any (pray).
---------------------------------------------------------------- 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
