API: Fix Metrics serialization #1430
Conversation
|
@rdblue: Could you please review this? |
| ByteBuffer bb = entry.getValue(); | ||
| byte[] bytes = new byte[bb.remaining()]; | ||
| bb.get(bytes); | ||
| bb.position(bb.position() - bytes.length); // Restores the buffer position |
There was a problem hiding this comment.
We have a ByteBuffers util located in the core module at org.apache.iceberg.util.ByteBuffers that handles the appropriate copying and restoration of ByteBuffers.
Please consider using that for consistency and for easier refactoring of ByteBuffer issues in the future. Here's a link to that class: https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/util/ByteBuffers.java
There was a problem hiding this comment.
I agree with @kbendick. You can call ByteBuffers.toByteArray instead of adding the logic to read here.
There was a problem hiding this comment.
Thanks @kbendick! Have not noticed that class.
Had to refactor that and move it to the API package to reuse it. Please check if you agree with the move, or you think there is a better place for it.
Thanks,
Peter
|
Thanks for fixing this, @pvary! |
5 participants
Metrics implements Serializable, but contains ByteBuffers which are not serializable.
This PR makes sure that the Metrics object remain serializable