You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by bowenli86 <gi...@git.apache.org> on 2018/01/20 06:40:58 UTC
[GitHub] flink pull request #5323: [FLINK-8441] [State Backend] [RocksDB] change Rock...
GitHub user bowenli86 opened a pull request:
https://github.com/apache/flink/pull/5323
[FLINK-8441] [State Backend] [RocksDB] change RocksDBListState to serialize values and separators in stream to avoid extra bytes copying
## What is the purpose of the change
Currently, `RocksDBListState#update()` and `addAll()` will both serialize values into a list of bytes, manually merge the list of bytes with separators to an array of bytes, and then write to RocksDB. It results in extra step of copying bytes back and forth.
## Brief change log
Changed `RocksDBListState#update()` and `addAll()` to serialize values and separators directly into `keySerializationStream`, which avoids extra copying.
## Verifying this change
This change is already covered by existing tests, such as `RocksDBStateBackendTest`
## Does this pull request potentially affect one of the following parts:
none
## Documentation
none
cc @StefanRRichter
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bowenli86/flink FLINK-8441
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5323.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5323
----
commit ca1636c40276c6cb826eb8b0cd221f3d2d440315
Author: Bowen Li <bo...@...>
Date: 2018-01-02T19:21:28Z
update local branch
commit e0cc43f051c76202416a72545dbf0cc024f718bc
Author: Bowen Li <bo...@...>
Date: 2018-01-04T01:35:11Z
remove sh
commit e5fcd4d844ac904cad9f25d3337554128b085363
Author: Bowen Li <bo...@...>
Date: 2018-01-20T06:19:06Z
[FLINK-8441] change RocksDBListState to serialize values and separators in stream to avoid extra bytes copying
----
---
[GitHub] flink issue #5323: [FLINK-8441] [State Backend] [RocksDB] change RocksDBList...
Posted by bowenli86 <gi...@git.apache.org>.
Github user bowenli86 commented on the issue:
https://github.com/apache/flink/pull/5323
Make sense. Updated the PR also squashed all commits
---
[GitHub] flink pull request #5323: [FLINK-8441] [State Backend] [RocksDB] change Rock...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5323
---
[GitHub] flink pull request #5323: [FLINK-8441] [State Backend] [RocksDB] change Rock...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5323#discussion_r162898707
--- Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBListState.java ---
@@ -60,6 +59,11 @@
*/
private final WriteOptions writeOptions;
+ /**
+ * Simulated separator of StringAppendTestOperator in RocksDB.
--- End diff --
I would drop the word `Simulated`.
---
[GitHub] flink issue #5323: [FLINK-8441] [State Backend] [RocksDB] change RocksDBList...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5323
Thanks for your contribution, I will merge this with some small cleanup.
---
[GitHub] flink pull request #5323: [FLINK-8441] [State Backend] [RocksDB] change Rock...
Posted by StefanRRichter <gi...@git.apache.org>.
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5323#discussion_r162898798
--- Diff: flink-contrib/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBListState.java ---
@@ -202,13 +206,17 @@ public void addAll(List<V> values) throws Exception {
private byte[] getPreMergedValue(List<V> values) throws IOException {
DataOutputViewStreamWrapper out = new DataOutputViewStreamWrapper(keySerializationStream);
- List<byte[]> bytes = new ArrayList<>(values.size());
+ keySerializationStream.reset();
+ byte first = 0;
--- End diff --
I think `boolean` is a more appropriate type.
---