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-9070) Improve performance of RocksDBMapState.clear()
Date Thu, 10 May 2018 09:16:00 GMT

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

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

Github user sihuazhou commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5979#discussion_r187278034
  
    --- Diff: flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBMapState.java
---
    @@ -356,6 +369,20 @@ private UV deserializeUserValue(byte[] rawValueBytes, TypeSerializer<UV>
valueSe
     		return isNull ? null : valueSerializer.deserialize(in);
     	}
     
    +	private boolean startWithKeyPrefix(byte[] keyPrefixBytes, byte[] rawKeyBytes) {
    +		if (rawKeyBytes.length < keyPrefixBytes.length) {
    +			return false;
    +		}
    +
    +		for (int i = keyPrefixBytes.length; --i >= backend.getKeyGroupPrefixBytes(); ) {
    --- End diff --
    
    Ah, this is a function that simply polled out from the `RocksDBMapState#RocksDBMapIterator`,
in`RocksDBMapState#RocksDBMapIterator` it's name was `underSameKey()` , I didn't change any
code related to it's implementation. 
    
    But concern this loop, yes, I was the person that written this loop for this method(`underSameKey()`),
and your suggestion was the first version that I implemented it, but during the reviewing
by @StefanRRichter , he suggest that the current style, and I feel that looks more simpler,
so I made it into the current shape finally, personally I would't against the current version.Ah,
this is a function that simply polled out from the `RocksDBMapState#RocksDBMapIterator`, in`RocksDBMapState#RocksDBMapIterator`
it's name was `underSameKey()` , I didn't change any code related to it's implementation.

    
    But concern this loop, yes, I was the person that written this loop for this method(`underSameKey()`),
and your suggestion was the first version that I implemented it, but during the reviewing
by @StefanRRichter , he suggest that the current style, and I feel that looks more simpler,
so I made it into the current shape finally, personally I would't against the current version.


> Improve performance of RocksDBMapState.clear()
> ----------------------------------------------
>
>                 Key: FLINK-9070
>                 URL: https://issues.apache.org/jira/browse/FLINK-9070
>             Project: Flink
>          Issue Type: Improvement
>          Components: State Backends, Checkpointing
>    Affects Versions: 1.6.0
>            Reporter: Truong Duc Kien
>            Assignee: Sihua Zhou
>            Priority: Major
>             Fix For: 1.6.0
>
>
> Currently, RocksDBMapState.clear() is implemented by iterating over all the keys and
drop them one by one. This iteration can be quite slow with: 
>  * Large maps
>  * High-churn maps with a lot of tombstones
> There are a few methods to speed-up deletion for a range of keys, each with their own
caveats:
>  * DeleteRange: still experimental, likely buggy
>  * DeleteFilesInRange + CompactRange: only good for large ranges
>  
> Flink can also keep a list of inserted keys in-memory, then directly delete them without
having to iterate over the Rocksdb database again. 
>  
> Reference:
>  * [RocksDB article about range deletion|https://github.com/facebook/rocksdb/wiki/Delete-A-Range-Of-Keys]
>  * [Bug in DeleteRange|https://pingcap.com/blog/2017-09-08-rocksdbbug]
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message