From 3f7f70b1918acf456b12384d1e63ccc85394ddac Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Wed, 5 Aug 2020 18:47:37 +0100 Subject: [PATCH 01/14] key/value reverse operations --- .../internals/AbstractReadOnlyDecorator.java | 12 ++ .../kafka/streams/state/KeyValueStore.java | 4 +- .../streams/state/ReadOnlyKeyValueStore.java | 47 ++++++-- ...bstractMergedSortedCacheStoreIterator.java | 57 +++++++--- .../state/internals/BytesRangeValidator.java | 37 +++++++ .../state/internals/CachingKeyValueStore.java | 38 ++++--- .../ChangeLoggingKeyValueBytesStore.java | 11 ++ .../internals/CompositeKeyValueIterator.java | 3 +- .../CompositeReadOnlyKeyValueStore.java | 47 +++++++- .../internals/InMemoryKeyValueStore.java | 45 ++++---- .../state/internals/MemoryLRUCache.java | 16 +++ .../internals/MemoryNavigableLRUCache.java | 22 +++- ...SortedCacheKeyValueBytesStoreIterator.java | 8 +- ...MergedSortedCacheSessionStoreIterator.java | 2 +- .../MergedSortedCacheWindowStoreIterator.java | 2 +- ...ortedCacheWindowStoreKeyValueIterator.java | 2 +- .../state/internals/MeteredKeyValueStore.java | 15 +++ .../streams/state/internals/NamedCache.java | 17 ++- .../ReadOnlyKeyValueStoreFacade.java | 11 ++ .../internals/RocksDBPrefixIterator.java | 2 +- .../state/internals/RocksDBRangeIterator.java | 29 +++-- .../streams/state/internals/RocksDBStore.java | 43 ++++++-- .../internals/RocksDBTimestampedStore.java | 51 ++++++--- .../state/internals/RocksDbIterator.java | 8 +- .../streams/state/internals/ThreadCache.java | 24 ++-- .../TimestampedKeyValueStoreBuilder.java | 11 ++ .../internals/AbstractKeyValueStoreTest.java | 102 ++++++++++++++++- .../internals/CachingKeyValueStoreTest.java | 39 +++++++ .../CompositeReadOnlyKeyValueStoreTest.java | 103 ++++++++++++++++++ .../internals/InMemoryKeyValueStoreTest.java | 6 +- ...edCacheKeyValueBytesStoreIteratorTest.java | 98 +++++++++++++---- 31 files changed, 763 insertions(+), 149 deletions(-) create mode 100644 streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadOnlyDecorator.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadOnlyDecorator.java index a63cd992cbe63..7c622fc0146e1 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadOnlyDecorator.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadOnlyDecorator.java @@ -17,6 +17,7 @@ package org.apache.kafka.streams.processor.internals; import java.util.List; + import org.apache.kafka.streams.KeyValue; import org.apache.kafka.streams.kstream.Windowed; import org.apache.kafka.streams.processor.ProcessorContext; @@ -90,11 +91,22 @@ public KeyValueIterator range(final K from, return wrapped().range(from, to); } + @Override + public KeyValueIterator reverseRange(final K from, + final K to) { + return wrapped().reverseRange(from, to); + } + @Override public KeyValueIterator all() { return wrapped().all(); } + @Override + public KeyValueIterator reverseAll() { + return wrapped().reverseAll(); + } + @Override public long approximateNumEntries() { return wrapped().approximateNumEntries(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/KeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/KeyValueStore.java index b104ad488db3a..3af8d901d849e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/KeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/KeyValueStore.java @@ -32,7 +32,7 @@ public interface KeyValueStore extends StateStore, ReadOnlyKeyValueStore extends StateStore, ReadOnlyKeyValueStore * Please note that this contract defines the thread-safe read functionality only; it does not * guarantee anything about whether the actual instance is writable by another thread, or * whether it uses some locking mechanism under the hood. For this reason, making dependencies @@ -38,7 +38,7 @@ public interface ReadOnlyKeyValueStore { * * @param key The key to fetch * @return The value or null if no value is found. - * @throws NullPointerException If null is used for key. + * @throws NullPointerException If null is used for key. * @throws InvalidStateStoreException if the store is not initialized */ V get(K key); @@ -46,27 +46,60 @@ public interface ReadOnlyKeyValueStore { /** * Get an iterator over a given range of keys. This iterator must be closed after use. * The returned iterator must be safe from {@link java.util.ConcurrentModificationException}s - * and must not return null values. No ordering guarantees are provided. + * and must not return null values. + * Order is not guaranteed as bytes lexicographical ordering might not represent key order. + * * @param from The first key that could be in the range - * @param to The last key that could be in the range + * @param to The last key that could be in the range * @return The iterator for this range. - * @throws NullPointerException If null is used for from or to. + * @throws NullPointerException If null is used for from or to. * @throws InvalidStateStoreException if the store is not initialized */ KeyValueIterator range(K from, K to); + /** + * Get a reverse iterator over a given range of keys. This iterator must be closed after use. + * The returned iterator must be safe from {@link java.util.ConcurrentModificationException}s + * and must not return null values. + * Order is not guaranteed as bytes lexicographical ordering might not represent key order. + * + * @param from The last key that could be in the range + * @param to The first key that could be in the range + * @return The reverse iterator for this range. + * @throws NullPointerException If null is used for from or to. + * @throws InvalidStateStoreException if the store is not initialized + */ + default KeyValueIterator reverseRange(K from, K to) { + throw new UnsupportedOperationException(); + } + /** * Return an iterator over all keys in this store. This iterator must be closed after use. * The returned iterator must be safe from {@link java.util.ConcurrentModificationException}s - * and must not return null values. No ordering guarantees are provided. + * and must not return null values. + * Order is not guaranteed as bytes lexicographical ordering might not represent key order. + * * @return An iterator of all key/value pairs in the store. * @throws InvalidStateStoreException if the store is not initialized */ KeyValueIterator all(); /** - * Return an approximate count of key-value mappings in this store. + * Return a reverse iterator over all keys in this store. This iterator must be closed after use. + * The returned iterator must be safe from {@link java.util.ConcurrentModificationException}s + * and must not return null values. + * Order is not guaranteed as bytes lexicographical ordering might not represent key order. * + * @return An reverse iterator of all key/value pairs in the store. + * @throws InvalidStateStoreException if the store is not initialized + */ + default KeyValueIterator reverseAll() { + throw new UnsupportedOperationException(); + } + + /** + * Return an approximate count of key-value mappings in this store. + *

* The count is not guaranteed to be exact in order to accommodate stores * where an exact count is expensive to calculate. * diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java index 16bdbeb8449f4..412b5af4eb3dd 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java @@ -31,11 +31,14 @@ abstract class AbstractMergedSortedCacheStoreIterator implements KeyValueIterator { private final PeekingKeyValueIterator cacheIterator; private final KeyValueIterator storeIterator; + private final boolean reverse; AbstractMergedSortedCacheStoreIterator(final PeekingKeyValueIterator cacheIterator, - final KeyValueIterator storeIterator) { + final KeyValueIterator storeIterator, + final boolean reverse) { this.cacheIterator = cacheIterator; this.storeIterator = storeIterator; + this.reverse = reverse; } abstract int compare(final Bytes cacheKey, final KS storeKey); @@ -87,14 +90,26 @@ public KeyValue next() { } final int comparison = compare(nextCacheKey, nextStoreKey); - if (comparison > 0) { - return nextStoreValue(nextStoreKey); - } else if (comparison < 0) { - return nextCacheValue(nextCacheKey); + if (!reverse) { + if (comparison > 0) { + return nextStoreValue(nextStoreKey); + } else if (comparison < 0) { + return nextCacheValue(nextCacheKey); + } else { + // skip the same keyed element + storeIterator.next(); + return nextCacheValue(nextCacheKey); + } } else { - // skip the same keyed element - storeIterator.next(); - return nextCacheValue(nextCacheKey); + if (comparison < 0) { + return nextStoreValue(nextStoreKey); + } else if (comparison > 0) { + return nextCacheValue(nextCacheKey); + } else { + // skip the same keyed element + storeIterator.next(); + return nextCacheValue(nextCacheKey); + } } } @@ -136,14 +151,26 @@ public K peekNextKey() { } final int comparison = compare(nextCacheKey, nextStoreKey); - if (comparison > 0) { - return deserializeStoreKey(nextStoreKey); - } else if (comparison < 0) { - return deserializeCacheKey(nextCacheKey); + if (!reverse) { + if (comparison > 0) { + return deserializeStoreKey(nextStoreKey); + } else if (comparison < 0) { + return deserializeCacheKey(nextCacheKey); + } else { + // skip the same keyed element + storeIterator.next(); + return deserializeCacheKey(nextCacheKey); + } } else { - // skip the same keyed element - storeIterator.next(); - return deserializeCacheKey(nextCacheKey); + if (comparison < 0) { + return deserializeStoreKey(nextStoreKey); + } else if (comparison > 0) { + return deserializeCacheKey(nextCacheKey); + } else { + // skip the same keyed element + storeIterator.next(); + return deserializeCacheKey(nextCacheKey); + } } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java new file mode 100644 index 0000000000000..c55e18eea4a30 --- /dev/null +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.streams.state.internals; + +import org.apache.kafka.common.utils.Bytes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class BytesRangeValidator { + + private static final Logger LOG = LoggerFactory.getLogger(BytesRangeValidator.class); + + static boolean isInvalid(final Bytes from, final Bytes to) { + if (from.compareTo(to) > 0) { + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + "Note that the built-in numerical serdes do not follow this for negative numbers"); + return true; + } + return false; + } +} diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java index 2e54014d0e89a..b92cd3088b1b3 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java @@ -24,8 +24,6 @@ import org.apache.kafka.streams.processor.internals.ProcessorRecordContext; import org.apache.kafka.streams.state.KeyValueIterator; import org.apache.kafka.streams.state.KeyValueStore; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.LinkedList; import java.util.List; @@ -41,8 +39,6 @@ public class CachingKeyValueStore extends WrappedStateStore, byte[], byte[]> implements KeyValueStore, CachedStateStore { - private static final Logger LOG = LoggerFactory.getLogger(CachingKeyValueStore.class); - private CacheFlushListener flushListener; private boolean sendOldValues; private String cacheName; @@ -64,7 +60,6 @@ public void init(final ProcessorContext context, streamThread = Thread.currentThread(); } - @SuppressWarnings("unchecked") private void initInternal(final ProcessorContext context) { this.context = (InternalProcessorContext) context; @@ -239,17 +234,23 @@ private byte[] getInternal(final Bytes key) { @Override public KeyValueIterator range(final Bytes from, final Bytes to) { - if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + - "Note that the built-in numerical serdes do not follow this for negative numbers"); - return KeyValueIterators.emptyIterator(); - } + if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); validateStoreOpen(); final KeyValueIterator storeIterator = wrapped().range(from, to); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().range(cacheName, from, to); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + } + + @Override + public KeyValueIterator reverseRange(final Bytes from, + final Bytes to) { + if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); + + validateStoreOpen(); + final KeyValueIterator storeIterator = wrapped().reverseRange(from, to); + final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().reverseRange(cacheName, from, to); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); } @Override @@ -258,7 +259,16 @@ public KeyValueIterator all() { final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>(this.name(), wrapped().all()); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().all(cacheName); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + } + + @Override + public KeyValueIterator reverseAll() { + validateStoreOpen(); + final KeyValueIterator storeIterator = + new DelegatingPeekingKeyValueIterator<>(this.name(), wrapped().reverseAll()); + final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().reverseAll(cacheName); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); } @Override @@ -309,7 +319,7 @@ public void close() { ); if (!suppressed.isEmpty()) { throwSuppressed("Caught an exception while closing caching key value store for store " + name(), - suppressed); + suppressed); } } finally { lock.writeLock().unlock(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStore.java index 35f6d365891ab..236f21877ef2d 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/ChangeLoggingKeyValueBytesStore.java @@ -100,11 +100,22 @@ public KeyValueIterator range(final Bytes from, return wrapped().range(from, to); } + @Override + public KeyValueIterator reverseRange(final Bytes from, + final Bytes to) { + return wrapped().reverseRange(from, to); + } + @Override public KeyValueIterator all() { return wrapped().all(); } + @Override + public KeyValueIterator reverseAll() { + return wrapped().reverseAll(); + } + void log(final Bytes key, final byte[] value) { context.logChange(name(), key, value, context.timestamp()); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeKeyValueIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeKeyValueIterator.java index 4ac6fee2e6cec..1614f9f52a15a 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeKeyValueIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeKeyValueIterator.java @@ -50,8 +50,7 @@ public K peekNextKey() { @Override public boolean hasNext() { - while ((current == null || !current.hasNext()) - && storeIterator.hasNext()) { + while ((current == null || !current.hasNext()) && storeIterator.hasNext()) { close(); current = nextIteratorFunction.apply(storeIterator.next()); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStore.java index c790b89b13f65..54e5f1e4f243a 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStore.java @@ -79,7 +79,29 @@ public KeyValueIterator apply(final ReadOnlyKeyValueStore store) { } }; final List> stores = storeProvider.stores(storeName, storeType); - return new DelegatingPeekingKeyValueIterator<>(storeName, new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); + return new DelegatingPeekingKeyValueIterator<>( + storeName, + new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); + } + + @Override + public KeyValueIterator reverseRange(final K from, final K to) { + Objects.requireNonNull(from); + Objects.requireNonNull(to); + final NextIteratorFunction> nextIteratorFunction = new NextIteratorFunction>() { + @Override + public KeyValueIterator apply(final ReadOnlyKeyValueStore store) { + try { + return store.reverseRange(from, to); + } catch (final InvalidStateStoreException e) { + throw new InvalidStateStoreException("State store is not available anymore and may have been migrated to another instance; please re-discover its location from the state metadata."); + } + } + }; + final List> stores = storeProvider.stores(storeName, storeType); + return new DelegatingPeekingKeyValueIterator<>( + storeName, + new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); } @Override @@ -95,7 +117,27 @@ public KeyValueIterator apply(final ReadOnlyKeyValueStore store) { } }; final List> stores = storeProvider.stores(storeName, storeType); - return new DelegatingPeekingKeyValueIterator<>(storeName, new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); + return new DelegatingPeekingKeyValueIterator<>( + storeName, + new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); + } + + @Override + public KeyValueIterator reverseAll() { + final NextIteratorFunction> nextIteratorFunction = new NextIteratorFunction>() { + @Override + public KeyValueIterator apply(final ReadOnlyKeyValueStore store) { + try { + return store.reverseAll(); + } catch (final InvalidStateStoreException e) { + throw new InvalidStateStoreException("State store is not available anymore and may have been migrated to another instance; please re-discover its location from the state metadata."); + } + } + }; + final List> stores = storeProvider.stores(storeName, storeType); + return new DelegatingPeekingKeyValueIterator<>( + storeName, + new CompositeKeyValueIterator<>(stores.iterator(), nextIteratorFunction)); } @Override @@ -111,6 +153,5 @@ public long approximateNumEntries() { return total; } - } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index 27ec409dc4d31..889e08c45b34d 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -16,11 +16,6 @@ */ package org.apache.kafka.streams.state.internals; -import java.util.List; -import java.util.NavigableMap; -import java.util.Set; -import java.util.TreeMap; -import java.util.TreeSet; import org.apache.kafka.common.utils.Bytes; import org.apache.kafka.streams.KeyValue; import org.apache.kafka.streams.processor.ProcessorContext; @@ -29,8 +24,11 @@ import org.apache.kafka.streams.state.KeyValueStore; import java.util.Iterator; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.util.List; +import java.util.NavigableMap; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; public class InMemoryKeyValueStore implements KeyValueStore { private final String name; @@ -38,8 +36,6 @@ public class InMemoryKeyValueStore implements KeyValueStore { private volatile boolean open = false; private long size = 0L; // SkipListMap#size is O(N) so we just do our best to track it - private static final Logger LOG = LoggerFactory.getLogger(InMemoryKeyValueStore.class); - public InMemoryKeyValueStore(final String name) { this.name = name; } @@ -110,24 +106,34 @@ public synchronized byte[] delete(final Bytes key) { @Override public synchronized KeyValueIterator range(final Bytes from, final Bytes to) { + return range(from, to, false); + } - if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + - "Note that the built-in numerical serdes do not follow this for negative numbers"); - return KeyValueIterators.emptyIterator(); - } + @Override + public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { + return range(from, to, true); + } + + KeyValueIterator range(final Bytes from, final Bytes to, final boolean reverse) { + if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); return new DelegatingPeekingKeyValueIterator<>( name, - new InMemoryKeyValueIterator(map.subMap(from, true, to, true).keySet())); + new InMemoryKeyValueIterator(map.subMap(from, true, to, true).keySet(), reverse)); } @Override public synchronized KeyValueIterator all() { return new DelegatingPeekingKeyValueIterator<>( name, - new InMemoryKeyValueIterator(map.keySet())); + new InMemoryKeyValueIterator(map.keySet(), false)); + } + + @Override + public KeyValueIterator reverseAll() { + return new DelegatingPeekingKeyValueIterator<>( + name, + new InMemoryKeyValueIterator(map.keySet(), true)); } @Override @@ -150,8 +156,9 @@ public void close() { private class InMemoryKeyValueIterator implements KeyValueIterator { private final Iterator iter; - private InMemoryKeyValueIterator(final Set keySet) { - this.iter = new TreeSet<>(keySet).iterator(); + private InMemoryKeyValueIterator(final Set keySet, final boolean reverse) { + if (reverse) this.iter = new TreeSet<>(keySet).descendingIterator(); + else this.iter = new TreeSet<>(keySet).iterator(); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryLRUCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryLRUCache.java index d69df13f4f096..32a91cd671299 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryLRUCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryLRUCache.java @@ -142,6 +142,14 @@ public KeyValueIterator range(final Bytes from, final Bytes to) { throw new UnsupportedOperationException("MemoryLRUCache does not support range() function."); } + /** + * @throws UnsupportedOperationException at every invocation + */ + @Override + public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { + throw new UnsupportedOperationException("MemoryLRUCache does not support reverseRange() function."); + } + /** * @throws UnsupportedOperationException at every invocation */ @@ -150,6 +158,14 @@ public KeyValueIterator all() { throw new UnsupportedOperationException("MemoryLRUCache does not support all() function."); } + /** + * @throws UnsupportedOperationException at every invocation + */ + @Override + public KeyValueIterator reverseAll() { + throw new UnsupportedOperationException("MemoryLRUCache does not support reverseAll() function."); + } + @Override public long approximateNumEntries() { return this.map.size(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java index 6e0deaa667eb9..7ce44206d9979 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java @@ -36,7 +36,6 @@ public MemoryNavigableLRUCache(final String name, final int maxCacheSize) { @Override public KeyValueIterator range(final Bytes from, final Bytes to) { - if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + @@ -50,12 +49,33 @@ public KeyValueIterator range(final Bytes from, final Bytes to) { .subSet(from, true, to, true).iterator(), treeMap)); } + @Override + public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { + if (from.compareTo(to) > 0) { + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + "Note that the built-in numerical serdes do not follow this for negative numbers"); + return KeyValueIterators.emptyIterator(); + } + + final TreeMap treeMap = toTreeMap(); + return new DelegatingPeekingKeyValueIterator<>(name(), + new MemoryNavigableLRUCache.CacheIterator(treeMap + .subMap(from, true, to, true).descendingKeySet().iterator(), treeMap)); + } + @Override public KeyValueIterator all() { final TreeMap treeMap = toTreeMap(); return new MemoryNavigableLRUCache.CacheIterator(treeMap.navigableKeySet().iterator(), treeMap); } + @Override + public KeyValueIterator reverseAll() { + final TreeMap treeMap = toTreeMap(); + return new MemoryNavigableLRUCache.CacheIterator(treeMap.descendingKeySet().iterator(), treeMap); + } + private synchronized TreeMap toTreeMap() { return new TreeMap<>(this.map); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java index 7a545e4da3f9b..5cb426edb08ca 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java @@ -24,12 +24,14 @@ * Merges two iterators. Assumes each of them is sorted by key * */ -class MergedSortedCacheKeyValueBytesStoreIterator extends AbstractMergedSortedCacheStoreIterator { +class MergedSortedCacheKeyValueBytesStoreIterator + extends AbstractMergedSortedCacheStoreIterator { MergedSortedCacheKeyValueBytesStoreIterator(final PeekingKeyValueIterator cacheIterator, - final KeyValueIterator storeIterator) { - super(cacheIterator, storeIterator); + final KeyValueIterator storeIterator, + final boolean reverse) { + super(cacheIterator, storeIterator, reverse); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java index 6994222531067..e50e4bbcfcca5 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java @@ -33,7 +33,7 @@ class MergedSortedCacheSessionStoreIterator extends AbstractMergedSortedCacheSto MergedSortedCacheSessionStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator, byte[]> storeIterator, final SegmentedCacheFunction cacheFunction) { - super(cacheIterator, storeIterator); + super(cacheIterator, storeIterator, false); this.cacheFunction = cacheFunction; } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java index 98b3f064db178..ebdabc9b5bc07 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java @@ -32,7 +32,7 @@ class MergedSortedCacheWindowStoreIterator extends AbstractMergedSortedCacheStor MergedSortedCacheWindowStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator storeIterator) { - super(cacheIterator, storeIterator); + super(cacheIterator, storeIterator, false); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java index 1cba018c285d6..c18abb1ba823c 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java @@ -37,7 +37,7 @@ class MergedSortedCacheWindowStoreKeyValueIterator final long windowSize, final SegmentedCacheFunction cacheFunction ) { - super(filteredCacheIterator, underlyingIterator); + super(filteredCacheIterator, underlyingIterator, false); this.serdes = serdes; this.windowSize = windowSize; this.cacheFunction = cacheFunction; diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java index d77834b66e65c..9fe5be998477e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java @@ -42,6 +42,7 @@ * inner KeyValueStore implementation do not need to provide its own metrics collecting functionality. * The inner {@link KeyValueStore} of this class is of type <Bytes,byte[]>, hence we use {@link Serde}s * to convert from <K,V> to <Bytes,byte[]> + * * @param * @param */ @@ -187,11 +188,25 @@ public KeyValueIterator range(final K from, ); } + @Override + public KeyValueIterator reverseRange(final K from, + final K to) { + return new MeteredKeyValueIterator( + wrapped().reverseRange(Bytes.wrap(serdes.rawKey(from)), Bytes.wrap(serdes.rawKey(to))), + rangeSensor + ); + } + @Override public KeyValueIterator all() { return new MeteredKeyValueIterator(wrapped().all(), allSensor); } + @Override + public KeyValueIterator reverseAll() { + return new MeteredKeyValueIterator(wrapped().reverseAll(), allSensor); + } + @Override public void flush() { maybeMeasureLatency(super::flush, time, flushSensor); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java index 4693fbc7b6540..be8e9009bd59f 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java @@ -281,15 +281,24 @@ public boolean isEmpty() { } synchronized Iterator keyRange(final Bytes from, final Bytes to) { - return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true)); + return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), false); } - private Iterator keySetIterator(final Set keySet) { - return new TreeSet<>(keySet).iterator(); + synchronized Iterator reverseKeyRange(final Bytes from, final Bytes to) { + return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), true); + } + + private Iterator keySetIterator(final Set keySet, final boolean reverse) { + if (reverse) return new TreeSet<>(keySet).descendingIterator(); + else return new TreeSet<>(keySet).iterator(); } synchronized Iterator allKeys() { - return keySetIterator(cache.navigableKeySet()); + return keySetIterator(cache.navigableKeySet(), false); + } + + synchronized Iterator reverseAllKeys() { + return keySetIterator(cache.navigableKeySet(), true); } synchronized LRUCacheEntry first() { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/ReadOnlyKeyValueStoreFacade.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/ReadOnlyKeyValueStoreFacade.java index 862e6fcfbe89e..2ffa3f3529863 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/ReadOnlyKeyValueStoreFacade.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/ReadOnlyKeyValueStoreFacade.java @@ -40,11 +40,22 @@ public KeyValueIterator range(final K from, return new KeyValueIteratorFacade<>(inner.range(from, to)); } + @Override + public KeyValueIterator reverseRange(final K from, + final K to) { + return new KeyValueIteratorFacade<>(inner.reverseRange(from, to)); + } + @Override public KeyValueIterator all() { return new KeyValueIteratorFacade<>(inner.all()); } + @Override + public KeyValueIterator reverseAll() { + return new KeyValueIteratorFacade<>(inner.reverseAll()); + } + @Override public long approximateNumEntries() { return inner.approximateNumEntries(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java index b84175e1ee0c6..303ceead0ffc0 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java @@ -29,7 +29,7 @@ class RocksDBPrefixIterator extends RocksDbIterator { final RocksIterator newIterator, final Set> openIterators, final Bytes prefix) { - super(name, newIterator, openIterators); + super(name, newIterator, openIterators, false); rawPrefix = prefix.get(); newIterator.seek(rawPrefix); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java index b1cf24dcdd1b5..66df25a0d3927 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java @@ -29,17 +29,25 @@ class RocksDBRangeIterator extends RocksDbIterator { // comparator to be pluggable, and the default is lexicographic, so it's // safe to just force lexicographic comparator here for now. private final Comparator comparator = Bytes.BYTES_LEXICO_COMPARATOR; - private final byte[] rawToKey; + private final byte[] rawLastKey; + private final boolean reverse; RocksDBRangeIterator(final String storeName, final RocksIterator iter, final Set> openIterators, final Bytes from, - final Bytes to) { - super(storeName, iter, openIterators); - iter.seek(from.get()); - rawToKey = to.get(); - if (rawToKey == null) { + final Bytes to, + final boolean reverse) { + super(storeName, iter, openIterators, reverse); + this.reverse = reverse; + if (reverse) { + iter.seekForPrev(to.get()); + rawLastKey = from.get(); + } else { + iter.seek(from.get()); + rawLastKey = to.get(); + } + if (rawLastKey == null) { throw new NullPointerException("RocksDBRangeIterator: RawToKey is null for key " + to); } } @@ -47,14 +55,15 @@ class RocksDBRangeIterator extends RocksDbIterator { @Override public KeyValue makeNext() { final KeyValue next = super.makeNext(); - if (next == null) { return allDone(); } else { - if (comparator.compare(next.key.get(), rawToKey) <= 0) { - return next; + if (!reverse) { + if (comparator.compare(next.key.get(), rawLastKey) <= 0) return next; + else return allDone(); } else { - return allDone(); + if (comparator.compare(next.key.get(), rawLastKey) >= 0) return next; + else return allDone(); } } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java index 0bc256f83c4ec..b9af3628d7c31 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java @@ -327,6 +327,18 @@ public synchronized byte[] delete(final Bytes key) { @Override public synchronized KeyValueIterator range(final Bytes from, final Bytes to) { + return range(from, to, false); + } + + @Override + public synchronized KeyValueIterator reverseRange(final Bytes from, + final Bytes to) { + return range(from, to, true); + } + + KeyValueIterator range(final Bytes from, + final Bytes to, + final boolean reverse) { Objects.requireNonNull(from, "from cannot be null"); Objects.requireNonNull(to, "to cannot be null"); @@ -339,7 +351,7 @@ public synchronized KeyValueIterator range(final Bytes from, validateStoreOpen(); - final KeyValueIterator rocksDBRangeIterator = dbAccessor.range(from, to); + final KeyValueIterator rocksDBRangeIterator = dbAccessor.range(from, to, reverse); openIterators.add(rocksDBRangeIterator); return rocksDBRangeIterator; @@ -347,8 +359,17 @@ public synchronized KeyValueIterator range(final Bytes from, @Override public synchronized KeyValueIterator all() { + return all(false); + } + + @Override + public KeyValueIterator reverseAll() { + return all(true); + } + + KeyValueIterator all(final boolean reverse) { validateStoreOpen(); - final KeyValueIterator rocksDbIterator = dbAccessor.all(); + final KeyValueIterator rocksDbIterator = dbAccessor.all(reverse); openIterators.add(rocksDbIterator); return rocksDbIterator; } @@ -474,9 +495,10 @@ void prepareBatch(final List> entries, byte[] getOnly(final byte[] key) throws RocksDBException; KeyValueIterator range(final Bytes from, - final Bytes to); + final Bytes to, + final boolean reverse); - KeyValueIterator all(); + KeyValueIterator all(final boolean reverse); long approximateNumEntries() throws RocksDBException; @@ -540,20 +562,23 @@ public byte[] getOnly(final byte[] key) throws RocksDBException { @Override public KeyValueIterator range(final Bytes from, - final Bytes to) { + final Bytes to, + final boolean reverse) { return new RocksDBRangeIterator( name, db.newIterator(columnFamily), openIterators, from, - to); + to, + reverse); } @Override - public KeyValueIterator all() { + public KeyValueIterator all(final boolean reverse) { final RocksIterator innerIterWithTimestamp = db.newIterator(columnFamily); - innerIterWithTimestamp.seekToFirst(); - return new RocksDbIterator(name, innerIterWithTimestamp, openIterators); + if (reverse) innerIterWithTimestamp.seekToLast(); + else innerIterWithTimestamp.seekToFirst(); + return new RocksDbIterator(name, innerIterWithTimestamp, openIterators, reverse); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java index 6c31e9b43d208..311b62ff377a1 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java @@ -193,22 +193,26 @@ public byte[] getOnly(final byte[] key) throws RocksDBException { @Override public KeyValueIterator range(final Bytes from, - final Bytes to) { + final Bytes to, + final boolean reverse) { return new RocksDBDualCFRangeIterator( name, db.newIterator(newColumnFamily), db.newIterator(oldColumnFamily), from, - to); + to, + reverse); } @Override - public KeyValueIterator all() { + public KeyValueIterator all(final boolean reverse) { final RocksIterator innerIterWithTimestamp = db.newIterator(newColumnFamily); - innerIterWithTimestamp.seekToFirst(); + if (reverse) innerIterWithTimestamp.seekToLast(); + else innerIterWithTimestamp.seekToFirst(); final RocksIterator innerIterNoTimestamp = db.newIterator(oldColumnFamily); - innerIterNoTimestamp.seekToFirst(); - return new RocksDBDualCFIterator(name, innerIterWithTimestamp, innerIterNoTimestamp); + if (reverse) innerIterNoTimestamp.seekToLast(); + else innerIterNoTimestamp.seekToFirst(); + return new RocksDBDualCFIterator(name, innerIterWithTimestamp, innerIterNoTimestamp, reverse); } @Override @@ -262,6 +266,7 @@ private class RocksDBDualCFIterator extends AbstractIterator makeNext() { } else { next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); nextWithTimestamp = null; - iterWithTimestamp.next(); + if (reverse) iterWithTimestamp.prev(); + else iterWithTimestamp.next(); } } else { if (nextWithTimestamp == null) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; - iterNoTimestamp.next(); + if (reverse) iterNoTimestamp.prev(); + else iterNoTimestamp.next(); } else { if (comparator.compare(nextNoTimestamp, nextWithTimestamp) <= 0) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; - iterNoTimestamp.next(); + if (reverse) iterNoTimestamp.prev(); + else iterNoTimestamp.next(); } else { next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); nextWithTimestamp = null; - iterWithTimestamp.next(); + if (reverse) iterWithTimestamp.prev(); + else iterWithTimestamp.next(); } } } - return next; } @@ -357,11 +367,18 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { final RocksIterator iterWithTimestamp, final RocksIterator iterNoTimestamp, final Bytes from, - final Bytes to) { - super(storeName, iterWithTimestamp, iterNoTimestamp); - iterWithTimestamp.seek(from.get()); - iterNoTimestamp.seek(from.get()); - upperBoundKey = to.get(); + final Bytes to, + final boolean reverse) { + super(storeName, iterWithTimestamp, iterNoTimestamp, reverse); + if (reverse) { + iterWithTimestamp.seek(to.get()); + iterNoTimestamp.seek(to.get()); + upperBoundKey = from.get(); + } else { + iterWithTimestamp.seek(from.get()); + iterNoTimestamp.seek(from.get()); + upperBoundKey = to.get(); + } if (upperBoundKey == null) { throw new NullPointerException("RocksDBDualCFRangeIterator: upperBoundKey is null for key " + to); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java index 9fa747a45b9c0..23087baaa5024 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java @@ -31,6 +31,7 @@ class RocksDbIterator extends AbstractIterator> implemen private final String storeName; private final RocksIterator iter; private final Set> openIterators; + private final boolean reverse; private volatile boolean open = true; @@ -38,10 +39,12 @@ class RocksDbIterator extends AbstractIterator> implemen RocksDbIterator(final String storeName, final RocksIterator iter, - final Set> openIterators) { + final Set> openIterators, + final boolean reverse) { this.storeName = storeName; this.iter = iter; this.openIterators = openIterators; + this.reverse = reverse; } @Override @@ -58,7 +61,8 @@ public KeyValue makeNext() { return allDone(); } else { next = getKeyValue(); - iter.next(); + if (reverse) iter.prev(); + else iter.next(); return next; } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/ThreadCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/ThreadCache.java index 0179536b63a28..594dc4616b6c0 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/ThreadCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/ThreadCache.java @@ -76,7 +76,6 @@ public long flushes() { * underlying store name. This method creates those names. * @param taskIDString Task ID * @param underlyingStoreName Underlying store name - * @return */ public static String nameSpaceFromTaskIdAndStore(final String taskIDString, final String underlyingStoreName) { return taskIDString + "-" + underlyingStoreName; @@ -84,8 +83,6 @@ public static String nameSpaceFromTaskIdAndStore(final String taskIDString, fina /** * Given a cache name of the form taskid-storename, return the task ID. - * @param cacheName - * @return */ public static String taskIDfromCacheName(final String cacheName) { final String[] tokens = cacheName.split("-", 2); @@ -94,8 +91,6 @@ public static String taskIDfromCacheName(final String cacheName) { /** * Given a cache name of the form taskid-storename, return the store name. - * @param cacheName - * @return */ public static String underlyingStoreNamefromCacheName(final String cacheName) { final String[] tokens = cacheName.split("-", 2); @@ -105,9 +100,6 @@ public static String underlyingStoreNamefromCacheName(final String cacheName) { /** * Add a listener that is called each time an entry is evicted from the cache or an explicit flush is called - * - * @param namespace - * @param listener */ public void addDirtyEntryFlushListener(final String namespace, final DirtyEntryFlushListener listener) { final NamedCache cache = getOrCreateCache(namespace); @@ -185,6 +177,14 @@ public MemoryLRUCacheBytesIterator range(final String namespace, final Bytes fro return new MemoryLRUCacheBytesIterator(cache.keyRange(from, to), cache); } + public MemoryLRUCacheBytesIterator reverseRange(final String namespace, final Bytes from, final Bytes to) { + final NamedCache cache = getCache(namespace); + if (cache == null) { + return new MemoryLRUCacheBytesIterator(Collections.emptyIterator(), new NamedCache(namespace, this.metrics)); + } + return new MemoryLRUCacheBytesIterator(cache.reverseKeyRange(from, to), cache); + } + public MemoryLRUCacheBytesIterator all(final String namespace) { final NamedCache cache = getCache(namespace); if (cache == null) { @@ -193,6 +193,14 @@ public MemoryLRUCacheBytesIterator all(final String namespace) { return new MemoryLRUCacheBytesIterator(cache.allKeys(), cache); } + public MemoryLRUCacheBytesIterator reverseAll(final String namespace) { + final NamedCache cache = getCache(namespace); + if (cache == null) { + return new MemoryLRUCacheBytesIterator(Collections.emptyIterator(), new NamedCache(namespace, this.metrics)); + } + return new MemoryLRUCacheBytesIterator(cache.reverseAllKeys(), cache); + } + public long size() { long size = 0; for (final NamedCache cache : caches.values()) { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyValueStoreBuilder.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyValueStoreBuilder.java index 863b44ba4f68e..be8f259366841 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyValueStoreBuilder.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/TimestampedKeyValueStoreBuilder.java @@ -133,11 +133,22 @@ public KeyValueIterator range(final Bytes from, return wrapped.range(from, to); } + @Override + public KeyValueIterator reverseRange(final Bytes from, + final Bytes to) { + return wrapped.reverseRange(from, to); + } + @Override public KeyValueIterator all() { return wrapped.all(); } + @Override + public KeyValueIterator reverseAll() { + return wrapped.reverseAll(); + } + @Override public long approximateNumEntries() { return wrapped.approximateNumEntries(); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java index ca5fd2fc7a840..9f6e70e6a13c4 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java @@ -40,12 +40,12 @@ import java.util.Map; import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public abstract class AbstractKeyValueStoreTest { @@ -188,7 +188,55 @@ public void testPutGetRange() { } @Test - public void testPutGetRangeWithDefaultSerdes() { + public void testPutGetReverseRange() { + // Verify that the store reads and writes correctly ... + store.put(0, "zero"); + store.put(1, "one"); + store.put(2, "two"); + store.put(4, "four"); + store.put(5, "five"); + assertEquals(5, driver.sizeOf(store)); + assertEquals("zero", store.get(0)); + assertEquals("one", store.get(1)); + assertEquals("two", store.get(2)); + assertNull(store.get(3)); + assertEquals("four", store.get(4)); + assertEquals("five", store.get(5)); + // Flush now so that for caching store, we will not skip the deletion following an put + store.flush(); + store.delete(5); + assertEquals(4, driver.sizeOf(store)); + + // Flush the store and verify all current entries were properly flushed ... + store.flush(); + assertEquals("zero", driver.flushedEntryStored(0)); + assertEquals("one", driver.flushedEntryStored(1)); + assertEquals("two", driver.flushedEntryStored(2)); + assertEquals("four", driver.flushedEntryStored(4)); + assertNull(driver.flushedEntryStored(5)); + + assertFalse(driver.flushedEntryRemoved(0)); + assertFalse(driver.flushedEntryRemoved(1)); + assertFalse(driver.flushedEntryRemoved(2)); + assertFalse(driver.flushedEntryRemoved(4)); + assertTrue(driver.flushedEntryRemoved(5)); + + final HashMap expectedContents = new HashMap<>(); + expectedContents.put(2, "two"); + expectedContents.put(4, "four"); + + // Check range iteration ... + assertEquals(expectedContents, getContents(store.reverseRange(2, 4))); + assertEquals(expectedContents, getContents(store.reverseRange(2, 6))); + + // Check all iteration ... + expectedContents.put(0, "zero"); + expectedContents.put(1, "one"); + assertEquals(expectedContents, getContents(store.reverseAll())); + } + + @Test + public void testPutGetWithDefaultSerdes() { // Verify that the store reads and writes correctly ... store.put(0, "zero"); store.put(1, "one"); @@ -371,7 +419,25 @@ public void shouldPutAll() { allReturned.add(iterator.next()); } assertThat(allReturned, equalTo(expectedReturned)); + } + + @Test + public void shouldPutReverseAll() { + final List> entries = new ArrayList<>(); + entries.add(new KeyValue<>(1, "one")); + entries.add(new KeyValue<>(2, "two")); + + store.putAll(entries); + final List> allReturned = new ArrayList<>(); + final List> expectedReturned = + Arrays.asList(KeyValue.pair(2, "two"), KeyValue.pair(1, "one")); + final Iterator> iterator = store.reverseAll(); + + while (iterator.hasNext()) { + allReturned.add(iterator.next()); + } + assertThat(allReturned, equalTo(expectedReturned)); } @Test @@ -397,6 +463,21 @@ public void shouldReturnSameResultsForGetAndRangeWithEqualKeys() { assertFalse(iterator.hasNext()); } + @Test + public void shouldReturnSameResultsForGetAndReverseRangeWithEqualKeys() { + final List> entries = new ArrayList<>(); + entries.add(new KeyValue<>(1, "one")); + entries.add(new KeyValue<>(2, "two")); + entries.add(new KeyValue<>(3, "three")); + + store.putAll(entries); + + final Iterator> iterator = store.reverseRange(2, 2); + + assertEquals(iterator.next().value, store.get(2)); + assertFalse(iterator.hasNext()); + } + @Test public void shouldNotThrowConcurrentModificationException() { store.put(0, "zero"); @@ -422,6 +503,21 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { " Note that the built-in numerical serdes do not follow this for negative numbers") ); } + } + + @Test + public void shouldNotThrowInvalidReverseRangeExceptionWithNegativeFromKey() { + try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister()) { + final KeyValueIterator iterator = store.reverseRange(-1, 1); + assertFalse(iterator.hasNext()); + final List messages = appender.getMessages(); + assertThat( + messages, + hasItem("Returning empty iterator for fetch with invalid key range: from > to." + + " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " Note that the built-in numerical serdes do not follow this for negative numbers") + ); + } } } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java index ffc8134a43a6c..9ce0d20a5d018 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java @@ -282,6 +282,17 @@ public void shouldIterateAllStoredItems() { assertEquals(items, results.size()); } + @Test + public void shouldReverseIterateAllStoredItems() { + final int items = addItemsToCache(); + final KeyValueIterator all = store.reverseAll(); + final List results = new ArrayList<>(); + while (all.hasNext()) { + results.add(all.next().key); + } + assertEquals(items, results.size()); + } + @Test public void shouldIterateOverRange() { final int items = addItemsToCache(); @@ -294,13 +305,27 @@ public void shouldIterateOverRange() { assertEquals(items, results.size()); } + @Test + public void shouldReverseIterateOverRange() { + final int items = addItemsToCache(); + final KeyValueIterator range = + store.reverseRange(bytesKey(String.valueOf(0)), bytesKey(String.valueOf(items))); + final List results = new ArrayList<>(); + while (range.hasNext()) { + results.add(range.next().key); + } + assertEquals(items, results.size()); + } + @Test public void shouldDeleteItemsFromCache() { store.put(bytesKey("a"), bytesValue("a")); store.delete(bytesKey("a")); assertNull(store.get(bytesKey("a"))); assertFalse(store.range(bytesKey("a"), bytesKey("b")).hasNext()); + assertFalse(store.reverseRange(bytesKey("a"), bytesKey("b")).hasNext()); assertFalse(store.all().hasNext()); + assertFalse(store.reverseAll().hasNext()); } @Test @@ -310,7 +335,9 @@ public void shouldNotShowItemsDeletedFromCacheButFlushedToStoreBeforeDelete() { store.delete(bytesKey("a")); assertNull(store.get(bytesKey("a"))); assertFalse(store.range(bytesKey("a"), bytesKey("b")).hasNext()); + assertFalse(store.reverseRange(bytesKey("a"), bytesKey("b")).hasNext()); assertFalse(store.all().hasNext()); + assertFalse(store.reverseAll().hasNext()); } @Test @@ -339,12 +366,24 @@ public void shouldThrowIfTryingToDoRangeQueryOnClosedCachingStore() { store.range(bytesKey("a"), bytesKey("b")); } + @Test(expected = InvalidStateStoreException.class) + public void shouldThrowIfTryingToDoReverseRangeQueryOnClosedCachingStore() { + store.close(); + store.reverseRange(bytesKey("a"), bytesKey("b")); + } + @Test(expected = InvalidStateStoreException.class) public void shouldThrowIfTryingToDoAllQueryOnClosedCachingStore() { store.close(); store.all(); } + @Test(expected = InvalidStateStoreException.class) + public void shouldThrowIfTryingToDoReverseAllQueryOnClosedCachingStore() { + store.close(); + store.reverseAll(); + } + @Test(expected = InvalidStateStoreException.class) public void shouldThrowIfTryingToDoGetApproxSizeOnClosedCachingStore() { store.close(); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java index 67b876ab48ed0..00312500fcc1b 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java @@ -41,6 +41,7 @@ import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.apache.kafka.test.StreamsTestUtils.toList; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -107,6 +108,16 @@ public void shouldThrowNullPointerExceptionOnRangeNullToKey() { theStore.range("from", null); } + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerExceptionOnReverseRangeNullFromKey() { + theStore.reverseRange(null, "to"); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerExceptionOnReverseRangeNullToKey() { + theStore.reverseRange("from", null); + } + @Test public void shouldReturnValueIfExists() { stubOneUnderlying.put("key", "value"); @@ -150,6 +161,17 @@ public void shouldThrowUnsupportedOperationExceptionWhileRemove() { } catch (final UnsupportedOperationException e) { } } + @Test + public void shouldThrowUnsupportedOperationExceptionWhileReverseRange() { + stubOneUnderlying.put("a", "1"); + stubOneUnderlying.put("b", "1"); + final KeyValueIterator keyValueIterator = theStore.reverseRange("a", "b"); + try { + keyValueIterator.remove(); + fail("Should have thrown UnsupportedOperationException"); + } catch (final UnsupportedOperationException e) { } + } + @Test public void shouldThrowUnsupportedOperationExceptionWhileRange() { stubOneUnderlying.put("a", "1"); @@ -185,6 +207,21 @@ public void shouldSupportRange() { assertEquals(2, results.size()); } + @Test + public void shouldSupportReverseRange() { + stubOneUnderlying.put("a", "a"); + stubOneUnderlying.put("b", "b"); + stubOneUnderlying.put("c", "c"); + + final List> results = toList(theStore.reverseRange("a", "b")); + assertArrayEquals( + asList( + new KeyValue<>("b", "b"), + new KeyValue<>("a", "a") + ).toArray(), + results.toArray()); + } + @Test public void shouldSupportRangeAcrossMultipleKVStores() { final KeyValueStore cache = newStoreInstance(); @@ -199,11 +236,44 @@ public void shouldSupportRangeAcrossMultipleKVStores() { cache.put("x", "x"); final List> results = toList(theStore.range("a", "e")); + assertArrayEquals( + asList( + new KeyValue<>("a", "a"), + new KeyValue<>("b", "b"), + new KeyValue<>("c", "c"), + new KeyValue<>("d", "d") + ).toArray(), + results.toArray()); + } + + @Test + public void shouldSupportReverseRangeAcrossMultipleKVStores() { + final KeyValueStore cache = newStoreInstance(); + stubProviderTwo.addStore(storeName, cache); + + stubOneUnderlying.put("a", "a"); + stubOneUnderlying.put("b", "b"); + stubOneUnderlying.put("z", "z"); + + cache.put("c", "c"); + cache.put("d", "d"); + cache.put("x", "x"); + + final List> results = toList(theStore.reverseRange("a", "e")); assertTrue(results.contains(new KeyValue<>("a", "a"))); assertTrue(results.contains(new KeyValue<>("b", "b"))); assertTrue(results.contains(new KeyValue<>("c", "c"))); assertTrue(results.contains(new KeyValue<>("d", "d"))); assertEquals(4, results.size()); + //FIXME: order does not hold between stores, how to validate order here? +// assertArrayEquals( +// asList( +// new KeyValue<>("d", "d"), +// new KeyValue<>("c", "c"), +// new KeyValue<>("b", "b"), +// new KeyValue<>("a", "a") +// ).toArray(), +// results.toArray()); } @Test @@ -229,6 +299,29 @@ public void shouldSupportAllAcrossMultipleStores() { assertEquals(6, results.size()); } + @Test + public void shouldSupportReverseAllAcrossMultipleStores() { + final KeyValueStore cache = newStoreInstance(); + stubProviderTwo.addStore(storeName, cache); + + stubOneUnderlying.put("a", "a"); + stubOneUnderlying.put("b", "b"); + stubOneUnderlying.put("z", "z"); + + cache.put("c", "c"); + cache.put("d", "d"); + cache.put("x", "x"); + + final List> results = toList(theStore.reverseAll()); + assertTrue(results.contains(new KeyValue<>("a", "a"))); + assertTrue(results.contains(new KeyValue<>("b", "b"))); + assertTrue(results.contains(new KeyValue<>("c", "c"))); + assertTrue(results.contains(new KeyValue<>("d", "d"))); + assertTrue(results.contains(new KeyValue<>("x", "x"))); + assertTrue(results.contains(new KeyValue<>("z", "z"))); + assertEquals(6, results.size()); + } + @Test(expected = InvalidStateStoreException.class) public void shouldThrowInvalidStoreExceptionDuringRebalance() { rebalancing().get("anything"); @@ -244,11 +337,21 @@ public void shouldThrowInvalidStoreExceptionOnRangeDuringRebalance() { rebalancing().range("anything", "something"); } + @Test(expected = InvalidStateStoreException.class) + public void shouldThrowInvalidStoreExceptionOnReverseRangeDuringRebalance() { + rebalancing().reverseRange("anything", "something"); + } + @Test(expected = InvalidStateStoreException.class) public void shouldThrowInvalidStoreExceptionOnAllDuringRebalance() { rebalancing().all(); } + @Test(expected = InvalidStateStoreException.class) + public void shouldThrowInvalidStoreExceptionOnReverseAllDuringRebalance() { + rebalancing().reverseAll(); + } + @Test public void shouldGetApproximateEntriesAcrossAllStores() { final KeyValueStore cache = newStoreInstance(); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStoreTest.java index 62f89495c7f9d..6dc90eac8d417 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStoreTest.java @@ -33,9 +33,9 @@ public class InMemoryKeyValueStoreTest extends AbstractKeyValueStoreTest { @Override protected KeyValueStore createKeyValueStore(final ProcessorContext context) { final StoreBuilder> storeBuilder = Stores.keyValueStoreBuilder( - Stores.inMemoryKeyValueStore("my-store"), - (Serde) context.keySerde(), - (Serde) context.valueSerde()); + Stores.inMemoryKeyValueStore("my-store"), + (Serde) context.keySerde(), + (Serde) context.valueSerde()); final KeyValueStore store = storeBuilder.build(); store.init(context, store); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java index 4028b0cd901f3..4fdca17da4373 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java @@ -17,13 +17,11 @@ package org.apache.kafka.streams.state.internals; import org.apache.kafka.common.metrics.Metrics; -import org.apache.kafka.common.serialization.Serdes; import org.apache.kafka.common.utils.Bytes; import org.apache.kafka.common.utils.LogContext; import org.apache.kafka.streams.processor.internals.MockStreamsMetrics; import org.apache.kafka.streams.state.KeyValueIterator; import org.apache.kafka.streams.state.KeyValueStore; -import org.apache.kafka.streams.state.StateSerdes; import org.junit.Before; import org.junit.Test; @@ -33,30 +31,30 @@ public class MergedSortedCacheKeyValueBytesStoreIteratorTest { private final String namespace = "0.0-one"; - private final StateSerdes serdes = new StateSerdes<>("dummy", Serdes.ByteArray(), Serdes.ByteArray()); private KeyValueStore store; private ThreadCache cache; @Before - public void setUp() throws Exception { + public void setUp() { store = new InMemoryKeyValueStore(namespace); cache = new ThreadCache(new LogContext("testCache "), 10000L, new MockStreamsMetrics(new Metrics())); } - @Test - public void shouldIterateOverRange() throws Exception { + public void shouldIterateOverRange() { final byte[][] bytes = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}, {11}}; for (int i = 0; i < bytes.length; i += 2) { store.put(Bytes.wrap(bytes[i]), bytes[i]); cache.put(namespace, Bytes.wrap(bytes[i + 1]), new LRUCacheEntry(bytes[i + 1])); } - final Bytes from = Bytes.wrap(new byte[]{2}); - final Bytes to = Bytes.wrap(new byte[]{9}); - final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>("store", store.range(from, to)); + final Bytes from = Bytes.wrap(new byte[] {2}); + final Bytes to = Bytes.wrap(new byte[] {9}); + final KeyValueIterator storeIterator = + new DelegatingPeekingKeyValueIterator<>("store", store.range(from, to)); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.range(namespace, from, to); - final MergedSortedCacheKeyValueBytesStoreIterator iterator = new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator); + final MergedSortedCacheKeyValueBytesStoreIterator iterator = + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 2; @@ -70,7 +68,34 @@ public void shouldIterateOverRange() throws Exception { @Test - public void shouldSkipLargerDeletedCacheValue() throws Exception { + public void shouldReverseIterateOverRange() { + final byte[][] bytes = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}, {11}}; + for (int i = 0; i < bytes.length; i += 2) { + store.put(Bytes.wrap(bytes[i]), bytes[i]); + cache.put(namespace, Bytes.wrap(bytes[i + 1]), new LRUCacheEntry(bytes[i + 1])); + } + + final Bytes from = Bytes.wrap(new byte[] {2}); + final Bytes to = Bytes.wrap(new byte[] {9}); + final KeyValueIterator storeIterator = + new DelegatingPeekingKeyValueIterator<>("store", store.reverseRange(from, to)); + final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.reverseRange(namespace, from, to); + + final MergedSortedCacheKeyValueBytesStoreIterator iterator = + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + final byte[][] values = new byte[8][]; + int index = 0; + int bytesIndex = 9; + while (iterator.hasNext()) { + final byte[] value = iterator.next().value; + values[index++] = value; + assertArrayEquals(bytes[bytesIndex--], value); + } + iterator.close(); + } + + @Test + public void shouldSkipLargerDeletedCacheValue() { final byte[][] bytes = {{0}, {1}}; store.put(Bytes.wrap(bytes[0]), bytes[0]); cache.put(namespace, Bytes.wrap(bytes[1]), new LRUCacheEntry(null)); @@ -80,7 +105,7 @@ public void shouldSkipLargerDeletedCacheValue() throws Exception { } @Test - public void shouldSkipSmallerDeletedCachedValue() throws Exception { + public void shouldSkipSmallerDeletedCachedValue() { final byte[][] bytes = {{0}, {1}}; cache.put(namespace, Bytes.wrap(bytes[0]), new LRUCacheEntry(null)); store.put(Bytes.wrap(bytes[1]), bytes[1]); @@ -90,7 +115,7 @@ public void shouldSkipSmallerDeletedCachedValue() throws Exception { } @Test - public void shouldIgnoreIfDeletedInCacheButExistsInStore() throws Exception { + public void shouldIgnoreIfDeletedInCacheButExistsInStore() { final byte[][] bytes = {{0}}; cache.put(namespace, Bytes.wrap(bytes[0]), new LRUCacheEntry(null)); store.put(Bytes.wrap(bytes[0]), bytes[0]); @@ -99,7 +124,7 @@ public void shouldIgnoreIfDeletedInCacheButExistsInStore() throws Exception { } @Test - public void shouldNotHaveNextIfAllCachedItemsDeleted() throws Exception { + public void shouldNotHaveNextIfAllCachedItemsDeleted() { final byte[][] bytes = {{0}, {1}, {2}}; for (final byte[] aByte : bytes) { final Bytes aBytes = Bytes.wrap(aByte); @@ -110,7 +135,7 @@ public void shouldNotHaveNextIfAllCachedItemsDeleted() throws Exception { } @Test - public void shouldNotHaveNextIfOnlyCacheItemsAndAllDeleted() throws Exception { + public void shouldNotHaveNextIfOnlyCacheItemsAndAllDeleted() { final byte[][] bytes = {{0}, {1}, {2}}; for (final byte[] aByte : bytes) { cache.put(namespace, Bytes.wrap(aByte), new LRUCacheEntry(null)); @@ -119,7 +144,7 @@ public void shouldNotHaveNextIfOnlyCacheItemsAndAllDeleted() throws Exception { } @Test - public void shouldSkipAllDeletedFromCache() throws Exception { + public void shouldSkipAllDeletedFromCache() { final byte[][] bytes = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}, {11}}; for (final byte[] aByte : bytes) { final Bytes aBytes = Bytes.wrap(aByte); @@ -145,7 +170,7 @@ public void shouldSkipAllDeletedFromCache() throws Exception { } @Test - public void shouldPeekNextKey() throws Exception { + public void shouldPeekNextKey() { final KeyValueStore kv = new InMemoryKeyValueStore("one"); final ThreadCache cache = new ThreadCache(new LogContext("testCache "), 1000000L, new MockStreamsMetrics(new Metrics())); final byte[][] bytes = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}}; @@ -154,15 +179,13 @@ public void shouldPeekNextKey() throws Exception { cache.put(namespace, Bytes.wrap(bytes[i + 1]), new LRUCacheEntry(bytes[i + 1])); } - final Bytes from = Bytes.wrap(new byte[]{2}); - final Bytes to = Bytes.wrap(new byte[]{9}); + final Bytes from = Bytes.wrap(new byte[] {2}); + final Bytes to = Bytes.wrap(new byte[] {9}); final KeyValueIterator storeIterator = kv.range(from, to); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.range(namespace, from, to); final MergedSortedCacheKeyValueBytesStoreIterator iterator = - new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, - storeIterator - ); + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 2; @@ -175,9 +198,38 @@ public void shouldPeekNextKey() throws Exception { iterator.close(); } + @Test + public void shouldPeekNextKeyReverse() { + final KeyValueStore kv = new InMemoryKeyValueStore("one"); + final ThreadCache cache = new ThreadCache(new LogContext("testCache "), 1000000L, new MockStreamsMetrics(new Metrics())); + final byte[][] bytes = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}}; + for (int i = 0; i < bytes.length - 1; i += 2) { + kv.put(Bytes.wrap(bytes[i]), bytes[i]); + cache.put(namespace, Bytes.wrap(bytes[i + 1]), new LRUCacheEntry(bytes[i + 1])); + } + + final Bytes from = Bytes.wrap(new byte[] {2}); + final Bytes to = Bytes.wrap(new byte[] {9}); + final KeyValueIterator storeIterator = kv.reverseRange(from, to); + final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.reverseRange(namespace, from, to); + + final MergedSortedCacheKeyValueBytesStoreIterator iterator = + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + final byte[][] values = new byte[8][]; + int index = 0; + int bytesIndex = 9; + while (iterator.hasNext()) { + final byte[] keys = iterator.peekNextKey().get(); + values[index++] = keys; + assertArrayEquals(bytes[bytesIndex--], keys); + iterator.next(); + } + iterator.close(); + } + private MergedSortedCacheKeyValueBytesStoreIterator createIterator() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.all(namespace); final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>("store", store.all()); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); } } \ No newline at end of file From 07acb80d25bcc934904079c8d00cd7fecc3df38e Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Wed, 5 Aug 2020 19:22:58 +0100 Subject: [PATCH 02/14] refactor choose next key and value --- .../internals/AbstractMergedSortedCacheStoreIterator.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java index 412b5af4eb3dd..7f2773de17c8f 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java @@ -90,6 +90,10 @@ public KeyValue next() { } final int comparison = compare(nextCacheKey, nextStoreKey); + return chooseNextValue(nextCacheKey, nextStoreKey, comparison); + } + + private KeyValue chooseNextValue(Bytes nextCacheKey, KS nextStoreKey, int comparison) { if (!reverse) { if (comparison > 0) { return nextStoreValue(nextStoreKey); @@ -151,6 +155,10 @@ public K peekNextKey() { } final int comparison = compare(nextCacheKey, nextStoreKey); + return chooseNextKey(nextCacheKey, nextStoreKey, comparison); + } + + private K chooseNextKey(Bytes nextCacheKey, KS nextStoreKey, int comparison) { if (!reverse) { if (comparison > 0) { return deserializeStoreKey(nextStoreKey); From 58f6c197be396a75c2a15dcffe6b8db07c6d483d Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Sat, 8 Aug 2020 10:19:00 +0100 Subject: [PATCH 03/14] fix final params --- .../streams/state/ReadOnlyKeyValueStore.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/ReadOnlyKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/ReadOnlyKeyValueStore.java index ff7602ccfa18c..37b85190c0f3f 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/ReadOnlyKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/ReadOnlyKeyValueStore.java @@ -49,9 +49,9 @@ public interface ReadOnlyKeyValueStore { * and must not return null values. * Order is not guaranteed as bytes lexicographical ordering might not represent key order. * - * @param from The first key that could be in the range - * @param to The last key that could be in the range - * @return The iterator for this range. + * @param from The first key that could be in the range, where iteration starts from. + * @param to The last key that could be in the range, where iteration ends. + * @return The iterator for this range, from smallest to largest bytes. * @throws NullPointerException If null is used for from or to. * @throws InvalidStateStoreException if the store is not initialized */ @@ -63,9 +63,9 @@ public interface ReadOnlyKeyValueStore { * and must not return null values. * Order is not guaranteed as bytes lexicographical ordering might not represent key order. * - * @param from The last key that could be in the range - * @param to The first key that could be in the range - * @return The reverse iterator for this range. + * @param from The first key that could be in the range, where iteration ends. + * @param to The last key that could be in the range, where iteration starts from. + * @return The reverse iterator for this range, from largest to smallest key bytes. * @throws NullPointerException If null is used for from or to. * @throws InvalidStateStoreException if the store is not initialized */ @@ -79,7 +79,7 @@ default KeyValueIterator reverseRange(K from, K to) { * and must not return null values. * Order is not guaranteed as bytes lexicographical ordering might not represent key order. * - * @return An iterator of all key/value pairs in the store. + * @return An iterator of all key/value pairs in the store, from smallest to largest bytes. * @throws InvalidStateStoreException if the store is not initialized */ KeyValueIterator all(); @@ -90,7 +90,7 @@ default KeyValueIterator reverseRange(K from, K to) { * and must not return null values. * Order is not guaranteed as bytes lexicographical ordering might not represent key order. * - * @return An reverse iterator of all key/value pairs in the store. + * @return An reverse iterator of all key/value pairs in the store, from largest to smallest key bytes. * @throws InvalidStateStoreException if the store is not initialized */ default KeyValueIterator reverseAll() { From e7ddbb9c78714995c4c3c5233b82626bf7da40a1 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Sat, 8 Aug 2020 10:19:29 +0100 Subject: [PATCH 04/14] improve ordering docs --- .../internals/AbstractMergedSortedCacheStoreIterator.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java index 7f2773de17c8f..d9e67beaaa296 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java @@ -93,7 +93,9 @@ public KeyValue next() { return chooseNextValue(nextCacheKey, nextStoreKey, comparison); } - private KeyValue chooseNextValue(Bytes nextCacheKey, KS nextStoreKey, int comparison) { + private KeyValue chooseNextValue(final Bytes nextCacheKey, + final KS nextStoreKey, + final int comparison) { if (!reverse) { if (comparison > 0) { return nextStoreValue(nextStoreKey); @@ -158,7 +160,9 @@ public K peekNextKey() { return chooseNextKey(nextCacheKey, nextStoreKey, comparison); } - private K chooseNextKey(Bytes nextCacheKey, KS nextStoreKey, int comparison) { + private K chooseNextKey(final Bytes nextCacheKey, + final KS nextStoreKey, + final int comparison) { if (!reverse) { if (comparison > 0) { return deserializeStoreKey(nextStoreKey); From 39d8e6339b6b268d27b295d378be5bce3dba4f5a Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Tue, 11 Aug 2020 13:47:57 +0100 Subject: [PATCH 05/14] fix range validator not needed --- .../state/internals/BytesRangeValidator.java | 37 ------------------- .../state/internals/CachingKeyValueStore.java | 18 ++++++++- .../internals/InMemoryKeyValueStore.java | 12 +++++- 3 files changed, 27 insertions(+), 40 deletions(-) delete mode 100644 streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java deleted file mode 100644 index c55e18eea4a30..0000000000000 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/BytesRangeValidator.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.kafka.streams.state.internals; - -import org.apache.kafka.common.utils.Bytes; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class BytesRangeValidator { - - private static final Logger LOG = LoggerFactory.getLogger(BytesRangeValidator.class); - - static boolean isInvalid(final Bytes from, final Bytes to) { - if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + - "Note that the built-in numerical serdes do not follow this for negative numbers"); - return true; - } - return false; - } -} diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java index b92cd3088b1b3..9c2bc93003eff 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java @@ -24,6 +24,8 @@ import org.apache.kafka.streams.processor.internals.ProcessorRecordContext; import org.apache.kafka.streams.state.KeyValueIterator; import org.apache.kafka.streams.state.KeyValueStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.LinkedList; import java.util.List; @@ -39,6 +41,8 @@ public class CachingKeyValueStore extends WrappedStateStore, byte[], byte[]> implements KeyValueStore, CachedStateStore { + private static final Logger LOG = LoggerFactory.getLogger(CachingKeyValueStore.class); + private CacheFlushListener flushListener; private boolean sendOldValues; private String cacheName; @@ -234,7 +238,12 @@ private byte[] getInternal(final Bytes key) { @Override public KeyValueIterator range(final Bytes from, final Bytes to) { - if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); + if (from.compareTo(to) > 0) { + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + "Note that the built-in numerical serdes do not follow this for negative numbers"); + return KeyValueIterators.emptyIterator(); + } validateStoreOpen(); final KeyValueIterator storeIterator = wrapped().range(from, to); @@ -245,7 +254,12 @@ public KeyValueIterator range(final Bytes from, @Override public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { - if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); + if (from.compareTo(to) > 0) { + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + "Note that the built-in numerical serdes do not follow this for negative numbers"); + return KeyValueIterators.emptyIterator(); + } validateStoreOpen(); final KeyValueIterator storeIterator = wrapped().reverseRange(from, to); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index 889e08c45b34d..4ab1d1994e48a 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -22,6 +22,8 @@ import org.apache.kafka.streams.processor.StateStore; import org.apache.kafka.streams.state.KeyValueIterator; import org.apache.kafka.streams.state.KeyValueStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Iterator; import java.util.List; @@ -31,6 +33,9 @@ import java.util.TreeSet; public class InMemoryKeyValueStore implements KeyValueStore { + + private static final Logger LOG = LoggerFactory.getLogger(InMemoryKeyValueStore.class); + private final String name; private final NavigableMap map = new TreeMap<>(); private volatile boolean open = false; @@ -115,7 +120,12 @@ public KeyValueIterator reverseRange(final Bytes from, final Byte } KeyValueIterator range(final Bytes from, final Bytes to, final boolean reverse) { - if (BytesRangeValidator.isInvalid(from, to)) return KeyValueIterators.emptyIterator(); + if (from.compareTo(to) > 0) { + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + "Note that the built-in numerical serdes do not follow this for negative numbers"); + return KeyValueIterators.emptyIterator(); + } return new DelegatingPeekingKeyValueIterator<>( name, From dddfa75530ee8939273d1eb9fe551d8ac9c20598 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Wed, 12 Aug 2020 13:32:08 +0100 Subject: [PATCH 06/14] resolve nits --- .../internals/InMemoryKeyValueStore.java | 9 ++- .../streams/state/internals/NamedCache.java | 7 ++- .../state/internals/RocksDBRangeIterator.java | 23 +++++--- .../streams/state/internals/RocksDBStore.java | 7 ++- .../internals/RocksDBTimestampedStore.java | 59 +++++++++++++------ .../state/internals/RocksDbIterator.java | 7 ++- 6 files changed, 77 insertions(+), 35 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index 4ab1d1994e48a..95b687817fa33 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -119,7 +119,7 @@ public KeyValueIterator reverseRange(final Bytes from, final Byte return range(from, to, true); } - KeyValueIterator range(final Bytes from, final Bytes to, final boolean reverse) { + private KeyValueIterator range(final Bytes from, final Bytes to, final boolean reverse) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + @@ -167,8 +167,11 @@ private class InMemoryKeyValueIterator implements KeyValueIterator iter; private InMemoryKeyValueIterator(final Set keySet, final boolean reverse) { - if (reverse) this.iter = new TreeSet<>(keySet).descendingIterator(); - else this.iter = new TreeSet<>(keySet).iterator(); + if (reverse) { + this.iter = new TreeSet<>(keySet).descendingIterator(); + } else { + this.iter = new TreeSet<>(keySet).iterator(); + } } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java index be8e9009bd59f..1346332aa1904 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java @@ -289,8 +289,11 @@ synchronized Iterator reverseKeyRange(final Bytes from, final Bytes to) { } private Iterator keySetIterator(final Set keySet, final boolean reverse) { - if (reverse) return new TreeSet<>(keySet).descendingIterator(); - else return new TreeSet<>(keySet).iterator(); + if (reverse) { + return new TreeSet<>(keySet).descendingIterator(); + } else { + return new TreeSet<>(keySet).iterator(); + } } synchronized Iterator allKeys() { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java index 66df25a0d3927..ca3194c5f1a02 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java @@ -43,12 +43,15 @@ class RocksDBRangeIterator extends RocksDbIterator { if (reverse) { iter.seekForPrev(to.get()); rawLastKey = from.get(); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBRangeIterator: RawLastKey is null for key " + from); + } } else { iter.seek(from.get()); rawLastKey = to.get(); - } - if (rawLastKey == null) { - throw new NullPointerException("RocksDBRangeIterator: RawToKey is null for key " + to); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBRangeIterator: RawLastKey is null for key " + to); + } } } @@ -59,11 +62,17 @@ public KeyValue makeNext() { return allDone(); } else { if (!reverse) { - if (comparator.compare(next.key.get(), rawLastKey) <= 0) return next; - else return allDone(); + if (comparator.compare(next.key.get(), rawLastKey) <= 0) { + return next; + } else { + return allDone(); + } } else { - if (comparator.compare(next.key.get(), rawLastKey) >= 0) return next; - else return allDone(); + if (comparator.compare(next.key.get(), rawLastKey) >= 0) { + return next; + } else { + return allDone(); + } } } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java index b9af3628d7c31..217fbfc770bc5 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java @@ -576,8 +576,11 @@ public KeyValueIterator range(final Bytes from, @Override public KeyValueIterator all(final boolean reverse) { final RocksIterator innerIterWithTimestamp = db.newIterator(columnFamily); - if (reverse) innerIterWithTimestamp.seekToLast(); - else innerIterWithTimestamp.seekToFirst(); + if (reverse) { + innerIterWithTimestamp.seekToLast(); + } else { + innerIterWithTimestamp.seekToFirst(); + } return new RocksDbIterator(name, innerIterWithTimestamp, openIterators, reverse); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java index 311b62ff377a1..e00cbccc1bb41 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java @@ -207,11 +207,17 @@ public KeyValueIterator range(final Bytes from, @Override public KeyValueIterator all(final boolean reverse) { final RocksIterator innerIterWithTimestamp = db.newIterator(newColumnFamily); - if (reverse) innerIterWithTimestamp.seekToLast(); - else innerIterWithTimestamp.seekToFirst(); + if (reverse) { + innerIterWithTimestamp.seekToLast(); + } else { + innerIterWithTimestamp.seekToFirst(); + } final RocksIterator innerIterNoTimestamp = db.newIterator(oldColumnFamily); - if (reverse) innerIterNoTimestamp.seekToLast(); - else innerIterNoTimestamp.seekToFirst(); + if (reverse) { + innerIterNoTimestamp.seekToLast(); + } else { + innerIterNoTimestamp.seekToFirst(); + } return new RocksDBDualCFIterator(name, innerIterWithTimestamp, innerIterNoTimestamp, reverse); } @@ -313,26 +319,38 @@ public KeyValue makeNext() { } else { next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); nextWithTimestamp = null; - if (reverse) iterWithTimestamp.prev(); - else iterWithTimestamp.next(); + if (reverse) { + iterWithTimestamp.prev(); + } else { + iterWithTimestamp.next(); + } } } else { if (nextWithTimestamp == null) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; - if (reverse) iterNoTimestamp.prev(); - else iterNoTimestamp.next(); + if (reverse) { + iterNoTimestamp.prev(); + } else { + iterNoTimestamp.next(); + } } else { if (comparator.compare(nextNoTimestamp, nextWithTimestamp) <= 0) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; - if (reverse) iterNoTimestamp.prev(); - else iterNoTimestamp.next(); + if (reverse) { + iterNoTimestamp.prev(); + } else { + iterNoTimestamp.next(); + } } else { next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); nextWithTimestamp = null; - if (reverse) iterWithTimestamp.prev(); - else iterWithTimestamp.next(); + if (reverse) { + iterWithTimestamp.prev(); + } else { + iterWithTimestamp.next(); + } } } } @@ -361,7 +379,7 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { // comparator to be pluggable, and the default is lexicographic, so it's // safe to just force lexicographic comparator here for now. private final Comparator comparator = Bytes.BYTES_LEXICO_COMPARATOR; - private final byte[] upperBoundKey; + private final byte[] lastKey; RocksDBDualCFRangeIterator(final String storeName, final RocksIterator iterWithTimestamp, @@ -373,14 +391,17 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { if (reverse) { iterWithTimestamp.seek(to.get()); iterNoTimestamp.seek(to.get()); - upperBoundKey = from.get(); + lastKey = from.get(); + if (lastKey == null) { + throw new NullPointerException("RocksDBDualCFRangeIterator: lastKey is null for key " + from); + } } else { iterWithTimestamp.seek(from.get()); iterNoTimestamp.seek(from.get()); - upperBoundKey = to.get(); - } - if (upperBoundKey == null) { - throw new NullPointerException("RocksDBDualCFRangeIterator: upperBoundKey is null for key " + to); + lastKey = to.get(); + if (lastKey == null) { + throw new NullPointerException("RocksDBDualCFRangeIterator: lastKey is null for key " + to); + } } } @@ -391,7 +412,7 @@ public KeyValue makeNext() { if (next == null) { return allDone(); } else { - if (comparator.compare(next.key.get(), upperBoundKey) <= 0) { + if (comparator.compare(next.key.get(), lastKey) <= 0) { return next; } else { return allDone(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java index 23087baaa5024..0472294dcdc5e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java @@ -61,8 +61,11 @@ public KeyValue makeNext() { return allDone(); } else { next = getKeyValue(); - if (reverse) iter.prev(); - else iter.next(); + if (reverse) { + iter.prev(); + } else { + iter.next(); + } return next; } } From 834da345e1336d9db801a3280c0a3c2869b9d4f8 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Wed, 12 Aug 2020 15:46:03 +0100 Subject: [PATCH 07/14] improve tests --- .../internals/AbstractKeyValueStoreTest.java | 32 +++++ .../internals/CachingKeyValueStoreTest.java | 127 +++++++++++------- 2 files changed, 114 insertions(+), 45 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java index 9f6e70e6a13c4..7345f8694083e 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java @@ -520,4 +520,36 @@ public void shouldNotThrowInvalidReverseRangeExceptionWithNegativeFromKey() { ); } } + + @Test + public void shouldNotThrowInvalidRangeExceptionWithFromLargerThanTo() { + try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister()) { + final KeyValueIterator iterator = store.range(2, 1); + assertFalse(iterator.hasNext()); + + final List messages = appender.getMessages(); + assertThat( + messages, + hasItem("Returning empty iterator for fetch with invalid key range: from > to." + + " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " Note that the built-in numerical serdes do not follow this for negative numbers") + ); + } + } + + @Test + public void shouldNotThrowInvalidReverseRangeExceptionWithFromLargerThanTo() { + try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister()) { + final KeyValueIterator iterator = store.reverseRange(2, 1); + assertFalse(iterator.hasNext()); + + final List messages = appender.getMessages(); + assertThat( + messages, + hasItem("Returning empty iterator for fetch with invalid key range: from > to." + + " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " Note that the built-in numerical serdes do not follow this for negative numbers") + ); + } + } } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java index 9ce0d20a5d018..d0e10d5367837 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingKeyValueStoreTest.java @@ -40,6 +40,7 @@ import org.junit.Test; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -54,7 +55,6 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class CachingKeyValueStoreTest extends AbstractKeyValueStoreTest { @@ -280,6 +280,11 @@ public void shouldIterateAllStoredItems() { results.add(all.next().key); } assertEquals(items, results.size()); + assertEquals(Arrays.asList( + Bytes.wrap("0".getBytes()), + Bytes.wrap("1".getBytes()), + Bytes.wrap("2".getBytes()) + ), results); } @Test @@ -291,6 +296,11 @@ public void shouldReverseIterateAllStoredItems() { results.add(all.next().key); } assertEquals(items, results.size()); + assertEquals(Arrays.asList( + Bytes.wrap("2".getBytes()), + Bytes.wrap("1".getBytes()), + Bytes.wrap("0".getBytes()) + ), results); } @Test @@ -303,6 +313,11 @@ public void shouldIterateOverRange() { results.add(range.next().key); } assertEquals(items, results.size()); + assertEquals(Arrays.asList( + Bytes.wrap("0".getBytes()), + Bytes.wrap("1".getBytes()), + Bytes.wrap("2".getBytes()) + ), results); } @Test @@ -315,6 +330,11 @@ public void shouldReverseIterateOverRange() { results.add(range.next().key); } assertEquals(items, results.size()); + assertEquals(Arrays.asList( + Bytes.wrap("2".getBytes()), + Bytes.wrap("1".getBytes()), + Bytes.wrap("0".getBytes()) + ), results); } @Test @@ -348,79 +368,94 @@ public void shouldClearNamespaceCacheOnClose() { assertEquals(0, cache.size()); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToGetFromClosedCachingStore() { - store.close(); - store.get(bytesKey("a")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.get(bytesKey("a")); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToWriteToClosedCachingStore() { - store.close(); - store.put(bytesKey("a"), bytesValue("a")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.put(bytesKey("a"), bytesValue("a")); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoRangeQueryOnClosedCachingStore() { - store.close(); - store.range(bytesKey("a"), bytesKey("b")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.range(bytesKey("a"), bytesKey("b")); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoReverseRangeQueryOnClosedCachingStore() { - store.close(); - store.reverseRange(bytesKey("a"), bytesKey("b")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.reverseRange(bytesKey("a"), bytesKey("b")); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoAllQueryOnClosedCachingStore() { - store.close(); - store.all(); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.all(); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoReverseAllQueryOnClosedCachingStore() { - store.close(); - store.reverseAll(); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.reverseAll(); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoGetApproxSizeOnClosedCachingStore() { - store.close(); - store.approximateNumEntries(); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.close(); + store.approximateNumEntries(); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoPutAllClosedCachingStore() { - store.close(); - store.putAll(Collections.singletonList(KeyValue.pair(bytesKey("a"), bytesValue("a")))); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.putAll(Collections.singletonList(KeyValue.pair(bytesKey("a"), bytesValue("a")))); + }); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDoPutIfAbsentClosedCachingStore() { - store.close(); - store.putIfAbsent(bytesKey("b"), bytesValue("c")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.putIfAbsent(bytesKey("b"), bytesValue("c")); + }); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnPutWithNullKey() { - store.put(null, bytesValue("c")); + assertThrows(NullPointerException.class, () -> store.put(null, bytesValue("c"))); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnPutIfAbsentWithNullKey() { - store.putIfAbsent(null, bytesValue("c")); + assertThrows(NullPointerException.class, () -> store.putIfAbsent(null, bytesValue("c"))); } @Test public void shouldThrowNullPointerExceptionOnPutAllWithNullKey() { final List> entries = new ArrayList<>(); entries.add(new KeyValue<>(null, bytesValue("a"))); - try { - store.putAll(entries); - fail("Should have thrown NullPointerException while putAll null key"); - } catch (final NullPointerException expected) { - } + assertThrows(NullPointerException.class, () -> store.putAll(entries)); } @Test @@ -447,10 +482,12 @@ public void shouldReturnUnderlying() { assertEquals(underlyingStore, store.wrapped()); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowIfTryingToDeleteFromClosedCachingStore() { - store.close(); - store.delete(bytesKey("key")); + assertThrows(InvalidStateStoreException.class, () -> { + store.close(); + store.delete(bytesKey("key")); + }); } private int addItemsToCache() { @@ -466,13 +503,13 @@ private int addItemsToCache() { public static class CacheFlushListenerStub implements CacheFlushListener { final Deserializer keyDeserializer; - final Deserializer valueDesializer; + final Deserializer valueDeserializer; final Map> forwarded = new HashMap<>(); CacheFlushListenerStub(final Deserializer keyDeserializer, - final Deserializer valueDesializer) { + final Deserializer valueDeserializer) { this.keyDeserializer = keyDeserializer; - this.valueDesializer = valueDesializer; + this.valueDeserializer = valueDeserializer; } @Override @@ -483,8 +520,8 @@ public void apply(final byte[] key, forwarded.put( keyDeserializer.deserialize(null, key), new Change<>( - valueDesializer.deserialize(null, newValue), - valueDesializer.deserialize(null, oldValue))); + valueDeserializer.deserialize(null, newValue), + valueDeserializer.deserialize(null, oldValue))); } } } From 842c358d5f657996f9398c168dcfab3c88c5e557 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Thu, 13 Aug 2020 13:16:01 +0100 Subject: [PATCH 08/14] improve tests --- .../CompositeReadOnlyKeyValueStoreTest.java | 107 +++++++----------- 1 file changed, 40 insertions(+), 67 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java index 00312500fcc1b..27dcff402faef 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CompositeReadOnlyKeyValueStoreTest.java @@ -21,16 +21,14 @@ import org.apache.kafka.streams.StoreQueryParameters; import org.apache.kafka.streams.errors.InvalidStateStoreException; import org.apache.kafka.streams.processor.internals.ProcessorStateManager; -import org.apache.kafka.streams.state.KeyValueStore; -import org.apache.kafka.streams.state.QueryableStoreType; -import org.apache.kafka.streams.state.Stores; -import org.apache.kafka.streams.state.StateSerdes; import org.apache.kafka.streams.state.KeyValueIterator; +import org.apache.kafka.streams.state.KeyValueStore; import org.apache.kafka.streams.state.QueryableStoreTypes; -import org.apache.kafka.streams.state.ReadOnlyKeyValueStore; +import org.apache.kafka.streams.state.StateSerdes; +import org.apache.kafka.streams.state.Stores; import org.apache.kafka.test.InternalMockProcessorContext; -import org.apache.kafka.test.NoOpReadOnlyStore; import org.apache.kafka.test.MockRecordCollector; +import org.apache.kafka.test.NoOpReadOnlyStore; import org.apache.kafka.test.StateStoreProviderStub; import org.junit.Before; import org.junit.Test; @@ -44,13 +42,12 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class CompositeReadOnlyKeyValueStoreTest { private final String storeName = "my-store"; - private final String storeNameA = "my-storeA"; private StateStoreProviderStub stubProviderTwo; private KeyValueStore stubOneUnderlying; private KeyValueStore otherUnderlyingStore; @@ -65,7 +62,6 @@ public void before() { stubProviderOne.addStore(storeName, stubOneUnderlying); otherUnderlyingStore = newStoreInstance(); stubProviderOne.addStore("other-store", otherUnderlyingStore); - final QueryableStoreType> queryableStoreType = QueryableStoreTypes.keyValueStore(); theStore = new CompositeReadOnlyKeyValueStore<>( new WrappingStoreProvider(asList(stubProviderOne, stubProviderTwo), StoreQueryParameters.fromNameAndType(storeName, QueryableStoreTypes.keyValueStore())), QueryableStoreTypes.keyValueStore(), @@ -75,9 +71,9 @@ public void before() { private KeyValueStore newStoreInstance() { final KeyValueStore store = Stores.keyValueStoreBuilder(Stores.inMemoryKeyValueStore(storeName), - Serdes.String(), - Serdes.String()) - .build(); + Serdes.String(), + Serdes.String()) + .build(); final InternalMockProcessorContext context = new InternalMockProcessorContext(new StateSerdes<>(ProcessorStateManager.storeChangelogTopic("appId", storeName), Serdes.String(), Serdes.String()), new MockRecordCollector()); @@ -89,33 +85,33 @@ private KeyValueStore newStoreInstance() { } @Test - public void shouldReturnNullIfKeyDoesntExist() { + public void shouldReturnNullIfKeyDoesNotExist() { assertNull(theStore.get("whatever")); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnGetNullKey() { - theStore.get(null); + assertThrows(NullPointerException.class, () -> theStore.get(null)); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnRangeNullFromKey() { - theStore.range(null, "to"); + assertThrows(NullPointerException.class, () -> theStore.range(null, "to")); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnRangeNullToKey() { - theStore.range("from", null); + assertThrows(NullPointerException.class, () -> theStore.range("from", null)); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnReverseRangeNullFromKey() { - theStore.reverseRange(null, "to"); + assertThrows(NullPointerException.class, () -> theStore.reverseRange(null, "to")); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowNullPointerExceptionOnReverseRangeNullToKey() { - theStore.reverseRange("from", null); + assertThrows(NullPointerException.class, () -> theStore.reverseRange("from", null)); } @Test @@ -135,10 +131,7 @@ public void shouldThrowNoSuchElementExceptionWhileNext() { stubOneUnderlying.put("a", "1"); final KeyValueIterator keyValueIterator = theStore.range("a", "b"); keyValueIterator.next(); - try { - keyValueIterator.next(); - fail("Should have thrown NoSuchElementException with next()"); - } catch (final NoSuchElementException e) { } + assertThrows(NoSuchElementException.class, keyValueIterator::next); } @Test @@ -146,19 +139,13 @@ public void shouldThrowNoSuchElementExceptionWhilePeekNext() { stubOneUnderlying.put("a", "1"); final KeyValueIterator keyValueIterator = theStore.range("a", "b"); keyValueIterator.next(); - try { - keyValueIterator.peekNextKey(); - fail("Should have thrown NoSuchElementException with peekNextKey()"); - } catch (final NoSuchElementException e) { } + assertThrows(NoSuchElementException.class, keyValueIterator::peekNextKey); } @Test public void shouldThrowUnsupportedOperationExceptionWhileRemove() { final KeyValueIterator keyValueIterator = theStore.all(); - try { - keyValueIterator.remove(); - fail("Should have thrown UnsupportedOperationException"); - } catch (final UnsupportedOperationException e) { } + assertThrows(UnsupportedOperationException.class, keyValueIterator::remove); } @Test @@ -166,10 +153,7 @@ public void shouldThrowUnsupportedOperationExceptionWhileReverseRange() { stubOneUnderlying.put("a", "1"); stubOneUnderlying.put("b", "1"); final KeyValueIterator keyValueIterator = theStore.reverseRange("a", "b"); - try { - keyValueIterator.remove(); - fail("Should have thrown UnsupportedOperationException"); - } catch (final UnsupportedOperationException e) { } + assertThrows(UnsupportedOperationException.class, keyValueIterator::remove); } @Test @@ -177,10 +161,7 @@ public void shouldThrowUnsupportedOperationExceptionWhileRange() { stubOneUnderlying.put("a", "1"); stubOneUnderlying.put("b", "1"); final KeyValueIterator keyValueIterator = theStore.range("a", "b"); - try { - keyValueIterator.remove(); - fail("Should have thrown UnsupportedOperationException"); - } catch (final UnsupportedOperationException e) { } + assertThrows(UnsupportedOperationException.class, keyValueIterator::remove); } @Test @@ -265,15 +246,6 @@ public void shouldSupportReverseRangeAcrossMultipleKVStores() { assertTrue(results.contains(new KeyValue<>("c", "c"))); assertTrue(results.contains(new KeyValue<>("d", "d"))); assertEquals(4, results.size()); - //FIXME: order does not hold between stores, how to validate order here? -// assertArrayEquals( -// asList( -// new KeyValue<>("d", "d"), -// new KeyValue<>("c", "c"), -// new KeyValue<>("b", "b"), -// new KeyValue<>("a", "a") -// ).toArray(), -// results.toArray()); } @Test @@ -322,34 +294,34 @@ public void shouldSupportReverseAllAcrossMultipleStores() { assertEquals(6, results.size()); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionDuringRebalance() { - rebalancing().get("anything"); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().get("anything")); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionOnApproximateNumEntriesDuringRebalance() { - rebalancing().approximateNumEntries(); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().approximateNumEntries()); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionOnRangeDuringRebalance() { - rebalancing().range("anything", "something"); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().range("anything", "something")); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionOnReverseRangeDuringRebalance() { - rebalancing().reverseRange("anything", "something"); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().reverseRange("anything", "something")); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionOnAllDuringRebalance() { - rebalancing().all(); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().all()); } - @Test(expected = InvalidStateStoreException.class) + @Test public void shouldThrowInvalidStoreExceptionOnReverseAllDuringRebalance() { - rebalancing().reverseAll(); + assertThrows(InvalidStateStoreException.class, () -> rebalancing().reverseAll()); } @Test @@ -389,7 +361,7 @@ public long approximateNumEntries() { return Long.MAX_VALUE; } }); - stubProviderTwo.addStore(storeNameA, new NoOpReadOnlyStore() { + stubProviderTwo.addStore("my-storeA", new NoOpReadOnlyStore() { @Override public long approximateNumEntries() { return Long.MAX_VALUE; @@ -400,9 +372,10 @@ public long approximateNumEntries() { } private CompositeReadOnlyKeyValueStore rebalancing() { - final QueryableStoreType> queryableStoreType = QueryableStoreTypes.keyValueStore(); return new CompositeReadOnlyKeyValueStore<>( - new WrappingStoreProvider(singletonList(new StateStoreProviderStub(true)), StoreQueryParameters.fromNameAndType(storeName, QueryableStoreTypes.keyValueStore())), + new WrappingStoreProvider( + singletonList(new StateStoreProviderStub(true)), + StoreQueryParameters.fromNameAndType(storeName, QueryableStoreTypes.keyValueStore())), QueryableStoreTypes.keyValueStore(), storeName ); From c634673602018315aebf2272273ffeacb5d93db7 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Mon, 17 Aug 2020 13:44:19 +0100 Subject: [PATCH 09/14] complete reverse paths --- .../internals/RocksDBPrefixIterator.java | 54 --------------- .../streams/state/internals/RocksDBStore.java | 3 - .../internals/RocksDBTimestampedStore.java | 61 ++++++++++------- .../state/internals/RocksDbIterator.java | 11 ++-- .../internals/RocksDBKeyValueStoreTest.java | 6 +- .../state/internals/RocksDBStoreTest.java | 6 +- .../RocksDBTimestampedStoreTest.java | 65 +++++++++++++++++++ 7 files changed, 113 insertions(+), 93 deletions(-) delete mode 100644 streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java deleted file mode 100644 index 303ceead0ffc0..0000000000000 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBPrefixIterator.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.kafka.streams.state.internals; - -import org.apache.kafka.common.utils.Bytes; -import org.apache.kafka.streams.state.KeyValueIterator; -import org.rocksdb.RocksIterator; - -import java.util.Set; - -class RocksDBPrefixIterator extends RocksDbIterator { - private byte[] rawPrefix; - - RocksDBPrefixIterator(final String name, - final RocksIterator newIterator, - final Set> openIterators, - final Bytes prefix) { - super(name, newIterator, openIterators, false); - rawPrefix = prefix.get(); - newIterator.seek(rawPrefix); - } - - @Override - public synchronized boolean hasNext() { - if (!super.hasNext()) { - return false; - } - - final byte[] rawNextKey = super.peekNextKey().get(); - for (int i = 0; i < rawPrefix.length; i++) { - if (i == rawNextKey.length) { - throw new IllegalStateException("Unexpected RocksDB Key Value. Should have been skipped with seek."); - } - if (rawNextKey[i] != rawPrefix[i]) { - return false; - } - } - return true; - } -} \ No newline at end of file diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java index 217fbfc770bc5..f1399f63572da 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java @@ -65,7 +65,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.regex.Pattern; import static org.apache.kafka.streams.StreamsConfig.METRICS_RECORDING_LEVEL_CONFIG; @@ -75,8 +74,6 @@ public class RocksDBStore implements KeyValueStore, BatchWritingStore { private static final Logger log = LoggerFactory.getLogger(RocksDBStore.class); - private static final Pattern SST_FILE_EXTENSION = Pattern.compile(".*\\.sst"); - private static final CompressionType COMPRESSION_TYPE = CompressionType.NO_COMPRESSION; private static final CompactionStyle COMPACTION_STYLE = CompactionStyle.UNIVERSAL; private static final long WRITE_BUFFER_SIZE = 16 * 1024 * 1024L; diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java index e00cbccc1bb41..2b932aa53d972 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java @@ -335,23 +335,28 @@ public KeyValue makeNext() { iterNoTimestamp.next(); } } else { - if (comparator.compare(nextNoTimestamp, nextWithTimestamp) <= 0) { - next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); - nextNoTimestamp = null; - if (reverse) { - iterNoTimestamp.prev(); - } else { + if (!reverse) { + if (comparator.compare(nextNoTimestamp, nextWithTimestamp) <= 0) { + next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); + nextNoTimestamp = null; iterNoTimestamp.next(); + } else { + next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); + nextWithTimestamp = null; + iterWithTimestamp.next(); } } else { - next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); - nextWithTimestamp = null; - if (reverse) { - iterWithTimestamp.prev(); + if (comparator.compare(nextNoTimestamp, nextWithTimestamp) >= 0) { + next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); + nextNoTimestamp = null; + iterNoTimestamp.prev(); } else { - iterWithTimestamp.next(); + next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); + nextWithTimestamp = null; + iterWithTimestamp.prev(); } } + } } return next; @@ -379,7 +384,8 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { // comparator to be pluggable, and the default is lexicographic, so it's // safe to just force lexicographic comparator here for now. private final Comparator comparator = Bytes.BYTES_LEXICO_COMPARATOR; - private final byte[] lastKey; + private final byte[] rawLastKey; + private final boolean reverse; RocksDBDualCFRangeIterator(final String storeName, final RocksIterator iterWithTimestamp, @@ -388,19 +394,20 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { final Bytes to, final boolean reverse) { super(storeName, iterWithTimestamp, iterNoTimestamp, reverse); + this.reverse = reverse; if (reverse) { - iterWithTimestamp.seek(to.get()); - iterNoTimestamp.seek(to.get()); - lastKey = from.get(); - if (lastKey == null) { - throw new NullPointerException("RocksDBDualCFRangeIterator: lastKey is null for key " + from); + iterWithTimestamp.seekForPrev(to.get()); + iterNoTimestamp.seekForPrev(to.get()); + rawLastKey = from.get(); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBDualCFRangeIterator: rawLastKey is null for key " + from); } } else { iterWithTimestamp.seek(from.get()); iterNoTimestamp.seek(from.get()); - lastKey = to.get(); - if (lastKey == null) { - throw new NullPointerException("RocksDBDualCFRangeIterator: lastKey is null for key " + to); + rawLastKey = to.get(); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBDualCFRangeIterator: rawLastKey is null for key " + to); } } } @@ -412,10 +419,18 @@ public KeyValue makeNext() { if (next == null) { return allDone(); } else { - if (comparator.compare(next.key.get(), lastKey) <= 0) { - return next; + if (!reverse) { + if (comparator.compare(next.key.get(), rawLastKey) <= 0) { + return next; + } else { + return allDone(); + } } else { - return allDone(); + if (comparator.compare(next.key.get(), rawLastKey) >= 0) { + return next; + } else { + return allDone(); + } } } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java index 0472294dcdc5e..5fe35e8c5ccf4 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java @@ -25,13 +25,14 @@ import java.util.NoSuchElementException; import java.util.Set; +import java.util.function.Consumer; class RocksDbIterator extends AbstractIterator> implements KeyValueIterator { private final String storeName; private final RocksIterator iter; private final Set> openIterators; - private final boolean reverse; + private final Consumer advanceIterator; private volatile boolean open = true; @@ -44,7 +45,7 @@ class RocksDbIterator extends AbstractIterator> implemen this.storeName = storeName; this.iter = iter; this.openIterators = openIterators; - this.reverse = reverse; + this.advanceIterator = reverse ? RocksIterator::prev : RocksIterator::next; } @Override @@ -61,11 +62,7 @@ public KeyValue makeNext() { return allDone(); } else { next = getKeyValue(); - if (reverse) { - iter.prev(); - } else { - iter.next(); - } + advanceIterator.accept(iter); return next; } } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBKeyValueStoreTest.java index 504aa9b760f19..5937af03121d0 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBKeyValueStoreTest.java @@ -40,9 +40,9 @@ public class RocksDBKeyValueStoreTest extends AbstractKeyValueStoreTest { @Override protected KeyValueStore createKeyValueStore(final ProcessorContext context) { final StoreBuilder> storeBuilder = Stores.keyValueStoreBuilder( - Stores.persistentKeyValueStore("my-store"), - (Serde) context.keySerde(), - (Serde) context.valueSerde()); + Stores.persistentKeyValueStore("my-store"), + (Serde) context.keySerde(), + (Serde) context.valueSerde()); final KeyValueStore store = storeBuilder.build(); store.init(context, store); diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java index a8e278f03071a..c61996cbf88d4 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java @@ -400,7 +400,7 @@ public void shouldHandleDeletesOnRestoreAll() { } @Test - public void shouldHandleDeletesAndPutbackOnRestoreAll() { + public void shouldHandleDeletesAndPutBackOnRestoreAll() { final List> entries = new ArrayList<>(); entries.add(new KeyValue<>("1".getBytes(UTF_8), "a".getBytes(UTF_8))); entries.add(new KeyValue<>("2".getBytes(UTF_8), "b".getBytes(UTF_8))); @@ -521,14 +521,14 @@ public void shouldThrowNullPointerExceptionOnRange() { () -> rocksDBStore.range(null, new Bytes(stringSerializer.serialize(null, "2")))); } - @Test(expected = ProcessorStateException.class) + @Test public void shouldThrowProcessorStateExceptionOnPutDeletedDir() throws IOException { rocksDBStore.init(context, rocksDBStore); Utils.delete(dir); rocksDBStore.put( new Bytes(stringSerializer.serialize(null, "anyKey")), stringSerializer.serialize(null, "anyValue")); - rocksDBStore.flush(); + assertThrows(ProcessorStateException.class, () -> rocksDBStore.flush()); } @Test diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStoreTest.java index 0108bbbf4816a..042039c87735a 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStoreTest.java @@ -271,6 +271,71 @@ private void iteratorsShouldNotMigrateData() { } assertFalse(it.hasNext()); } + + try (final KeyValueIterator itAll = rocksDBStore.reverseAll()) { + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key8".getBytes(), keyValue.key.get()); + assertArrayEquals(new byte[]{'t', 'i', 'm', 'e', 's', 't', 'a', 'm', 'p', '+', '8', '8', '8', '8', '8', '8', '8', '8'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key7".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 7777777 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '7', '7', '7', '7', '7', '7', '7'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key5".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 55555 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '5', '5', '5', '5', '5'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key4".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 4444 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '4', '4', '4', '4'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key2".getBytes(), keyValue.key.get()); + assertArrayEquals(new byte[]{'t', 'i', 'm', 'e', 's', 't', 'a', 'm', 'p', '+', '2', '2'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key11".getBytes(), keyValue.key.get()); + assertArrayEquals(new byte[]{'t', 'i', 'm', 'e', 's', 't', 'a', 'm', 'p', '+', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1'}, keyValue.value); + } + { + final KeyValue keyValue = itAll.next(); + assertArrayEquals("key1".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 1 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '1'}, keyValue.value); + } + assertFalse(itAll.hasNext()); + } + + try (final KeyValueIterator it = + rocksDBStore.reverseRange(new Bytes("key2".getBytes()), new Bytes("key5".getBytes()))) { + { + final KeyValue keyValue = it.next(); + assertArrayEquals("key5".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 55555 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '5', '5', '5', '5', '5'}, keyValue.value); + } + { + final KeyValue keyValue = it.next(); + assertArrayEquals("key4".getBytes(), keyValue.key.get()); + // unknown timestamp == -1 plus value == 4444 + assertArrayEquals(new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, '4', '4', '4', '4'}, keyValue.value); + } + { + final KeyValue keyValue = it.next(); + assertArrayEquals("key2".getBytes(), keyValue.key.get()); + assertArrayEquals(new byte[]{'t', 'i', 'm', 'e', 's', 't', 'a', 'm', 'p', '+', '2', '2'}, keyValue.value); + } + assertFalse(it.hasNext()); + } } private void verifyOldAndNewColumnFamily() throws Exception { From a8bdfd453b62b2b1c2c464d95504083348252909 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Tue, 18 Aug 2020 10:47:00 +0100 Subject: [PATCH 10/14] complete kvstore implementations --- .../internals/AbstractReadWriteDecorator.java | 11 +++++++++++ ...KeyValueToTimestampedKeyValueByteStoreAdapter.java | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadWriteDecorator.java b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadWriteDecorator.java index 494d98eb5bf40..61e47f6cf6c6e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadWriteDecorator.java +++ b/streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractReadWriteDecorator.java @@ -84,11 +84,22 @@ public KeyValueIterator range(final K from, return wrapped().range(from, to); } + @Override + public KeyValueIterator reverseRange(final K from, + final K to) { + return wrapped().reverseRange(from, to); + } + @Override public KeyValueIterator all() { return wrapped().all(); } + @Override + public KeyValueIterator reverseAll() { + return wrapped().reverseAll(); + } + @Override public long approximateNumEntries() { return wrapped().approximateNumEntries(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueToTimestampedKeyValueByteStoreAdapter.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueToTimestampedKeyValueByteStoreAdapter.java index 62cfac3b09ca2..fa29974c98330 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueToTimestampedKeyValueByteStoreAdapter.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/KeyValueToTimestampedKeyValueByteStoreAdapter.java @@ -118,11 +118,22 @@ public KeyValueIterator range(final Bytes from, return new KeyValueToTimestampedKeyValueIteratorAdapter<>(store.range(from, to)); } + @Override + public KeyValueIterator reverseRange(final Bytes from, + final Bytes to) { + return new KeyValueToTimestampedKeyValueIteratorAdapter<>(store.reverseRange(from, to)); + } + @Override public KeyValueIterator all() { return new KeyValueToTimestampedKeyValueIteratorAdapter<>(store.all()); } + @Override + public KeyValueIterator reverseAll() { + return new KeyValueToTimestampedKeyValueIteratorAdapter<>(store.reverseAll()); + } + @Override public long approximateNumEntries() { return store.approximateNumEntries(); From b124cb7b758941f957e23f20f87ee417a8055aea Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Thu, 20 Aug 2020 10:16:56 +0100 Subject: [PATCH 11/14] replace backward flag to forward flag --- ...bstractMergedSortedCacheStoreIterator.java | 10 +-- .../state/internals/CachingKeyValueStore.java | 8 +-- .../internals/InMemoryKeyValueStore.java | 20 +++--- ...SortedCacheKeyValueBytesStoreIterator.java | 4 +- ...MergedSortedCacheSessionStoreIterator.java | 2 +- .../MergedSortedCacheWindowStoreIterator.java | 2 +- ...ortedCacheWindowStoreKeyValueIterator.java | 2 +- .../streams/state/internals/NamedCache.java | 16 ++--- .../state/internals/RocksDBRangeIterator.java | 24 +++---- .../streams/state/internals/RocksDBStore.java | 34 +++++----- .../internals/RocksDBTimestampedStore.java | 68 +++++++++---------- .../state/internals/RocksDbIterator.java | 4 +- ...edCacheKeyValueBytesStoreIteratorTest.java | 10 +-- 13 files changed, 100 insertions(+), 104 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java index d9e67beaaa296..a5e56aa0d9ac0 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractMergedSortedCacheStoreIterator.java @@ -31,14 +31,14 @@ abstract class AbstractMergedSortedCacheStoreIterator implements KeyValueIterator { private final PeekingKeyValueIterator cacheIterator; private final KeyValueIterator storeIterator; - private final boolean reverse; + private final boolean forward; AbstractMergedSortedCacheStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator storeIterator, - final boolean reverse) { + final boolean forward) { this.cacheIterator = cacheIterator; this.storeIterator = storeIterator; - this.reverse = reverse; + this.forward = forward; } abstract int compare(final Bytes cacheKey, final KS storeKey); @@ -96,7 +96,7 @@ public KeyValue next() { private KeyValue chooseNextValue(final Bytes nextCacheKey, final KS nextStoreKey, final int comparison) { - if (!reverse) { + if (forward) { if (comparison > 0) { return nextStoreValue(nextStoreKey); } else if (comparison < 0) { @@ -163,7 +163,7 @@ public K peekNextKey() { private K chooseNextKey(final Bytes nextCacheKey, final KS nextStoreKey, final int comparison) { - if (!reverse) { + if (forward) { if (comparison > 0) { return deserializeStoreKey(nextStoreKey); } else if (comparison < 0) { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java index 9c2bc93003eff..fe818ac0ff102 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java @@ -248,7 +248,7 @@ public KeyValueIterator range(final Bytes from, validateStoreOpen(); final KeyValueIterator storeIterator = wrapped().range(from, to); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().range(cacheName, from, to); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); } @Override @@ -264,7 +264,7 @@ public KeyValueIterator reverseRange(final Bytes from, validateStoreOpen(); final KeyValueIterator storeIterator = wrapped().reverseRange(from, to); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().reverseRange(cacheName, from, to); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); } @Override @@ -273,7 +273,7 @@ public KeyValueIterator all() { final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>(this.name(), wrapped().all()); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().all(cacheName); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); } @Override @@ -282,7 +282,7 @@ public KeyValueIterator reverseAll() { final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>(this.name(), wrapped().reverseAll()); final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = context.cache().reverseAll(cacheName); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index 95b687817fa33..f219e4692991b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -111,15 +111,15 @@ public synchronized byte[] delete(final Bytes key) { @Override public synchronized KeyValueIterator range(final Bytes from, final Bytes to) { - return range(from, to, false); + return range(from, to, true); } @Override public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { - return range(from, to, true); + return range(from, to, false); } - private KeyValueIterator range(final Bytes from, final Bytes to, final boolean reverse) { + private KeyValueIterator range(final Bytes from, final Bytes to, final boolean forward) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + @@ -129,21 +129,21 @@ private KeyValueIterator range(final Bytes from, final Bytes to, return new DelegatingPeekingKeyValueIterator<>( name, - new InMemoryKeyValueIterator(map.subMap(from, true, to, true).keySet(), reverse)); + new InMemoryKeyValueIterator(map.subMap(from, true, to, true).keySet(), forward)); } @Override public synchronized KeyValueIterator all() { return new DelegatingPeekingKeyValueIterator<>( name, - new InMemoryKeyValueIterator(map.keySet(), false)); + new InMemoryKeyValueIterator(map.keySet(), true)); } @Override public KeyValueIterator reverseAll() { return new DelegatingPeekingKeyValueIterator<>( name, - new InMemoryKeyValueIterator(map.keySet(), true)); + new InMemoryKeyValueIterator(map.keySet(), false)); } @Override @@ -166,11 +166,11 @@ public void close() { private class InMemoryKeyValueIterator implements KeyValueIterator { private final Iterator iter; - private InMemoryKeyValueIterator(final Set keySet, final boolean reverse) { - if (reverse) { - this.iter = new TreeSet<>(keySet).descendingIterator(); - } else { + private InMemoryKeyValueIterator(final Set keySet, final boolean forward) { + if (forward) { this.iter = new TreeSet<>(keySet).iterator(); + } else { + this.iter = new TreeSet<>(keySet).descendingIterator(); } } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java index 5cb426edb08ca..701bdd1d4d548 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIterator.java @@ -30,8 +30,8 @@ class MergedSortedCacheKeyValueBytesStoreIterator MergedSortedCacheKeyValueBytesStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator storeIterator, - final boolean reverse) { - super(cacheIterator, storeIterator, reverse); + final boolean forward) { + super(cacheIterator, storeIterator, forward); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java index e50e4bbcfcca5..ff45a418889a9 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheSessionStoreIterator.java @@ -33,7 +33,7 @@ class MergedSortedCacheSessionStoreIterator extends AbstractMergedSortedCacheSto MergedSortedCacheSessionStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator, byte[]> storeIterator, final SegmentedCacheFunction cacheFunction) { - super(cacheIterator, storeIterator, false); + super(cacheIterator, storeIterator, true); this.cacheFunction = cacheFunction; } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java index ebdabc9b5bc07..7d40dda2bc5ce 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreIterator.java @@ -32,7 +32,7 @@ class MergedSortedCacheWindowStoreIterator extends AbstractMergedSortedCacheStor MergedSortedCacheWindowStoreIterator(final PeekingKeyValueIterator cacheIterator, final KeyValueIterator storeIterator) { - super(cacheIterator, storeIterator, false); + super(cacheIterator, storeIterator, true); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java index c18abb1ba823c..36e922fee4bb5 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MergedSortedCacheWindowStoreKeyValueIterator.java @@ -37,7 +37,7 @@ class MergedSortedCacheWindowStoreKeyValueIterator final long windowSize, final SegmentedCacheFunction cacheFunction ) { - super(filteredCacheIterator, underlyingIterator, false); + super(filteredCacheIterator, underlyingIterator, true); this.serdes = serdes; this.windowSize = windowSize; this.cacheFunction = cacheFunction; diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java index 1346332aa1904..032a303f617e2 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/NamedCache.java @@ -281,27 +281,27 @@ public boolean isEmpty() { } synchronized Iterator keyRange(final Bytes from, final Bytes to) { - return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), false); + return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), true); } synchronized Iterator reverseKeyRange(final Bytes from, final Bytes to) { - return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), true); + return keySetIterator(cache.navigableKeySet().subSet(from, true, to, true), false); } - private Iterator keySetIterator(final Set keySet, final boolean reverse) { - if (reverse) { - return new TreeSet<>(keySet).descendingIterator(); - } else { + private Iterator keySetIterator(final Set keySet, final boolean forward) { + if (forward) { return new TreeSet<>(keySet).iterator(); + } else { + return new TreeSet<>(keySet).descendingIterator(); } } synchronized Iterator allKeys() { - return keySetIterator(cache.navigableKeySet(), false); + return keySetIterator(cache.navigableKeySet(), true); } synchronized Iterator reverseAllKeys() { - return keySetIterator(cache.navigableKeySet(), true); + return keySetIterator(cache.navigableKeySet(), false); } synchronized LRUCacheEntry first() { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java index ca3194c5f1a02..29154860af3d2 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBRangeIterator.java @@ -30,28 +30,28 @@ class RocksDBRangeIterator extends RocksDbIterator { // safe to just force lexicographic comparator here for now. private final Comparator comparator = Bytes.BYTES_LEXICO_COMPARATOR; private final byte[] rawLastKey; - private final boolean reverse; + private final boolean forward; RocksDBRangeIterator(final String storeName, final RocksIterator iter, final Set> openIterators, final Bytes from, final Bytes to, - final boolean reverse) { - super(storeName, iter, openIterators, reverse); - this.reverse = reverse; - if (reverse) { - iter.seekForPrev(to.get()); - rawLastKey = from.get(); - if (rawLastKey == null) { - throw new NullPointerException("RocksDBRangeIterator: RawLastKey is null for key " + from); - } - } else { + final boolean forward) { + super(storeName, iter, openIterators, forward); + this.forward = forward; + if (forward) { iter.seek(from.get()); rawLastKey = to.get(); if (rawLastKey == null) { throw new NullPointerException("RocksDBRangeIterator: RawLastKey is null for key " + to); } + } else { + iter.seekForPrev(to.get()); + rawLastKey = from.get(); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBRangeIterator: RawLastKey is null for key " + from); + } } } @@ -61,7 +61,7 @@ public KeyValue makeNext() { if (next == null) { return allDone(); } else { - if (!reverse) { + if (forward) { if (comparator.compare(next.key.get(), rawLastKey) <= 0) { return next; } else { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java index f1399f63572da..73649f566e35a 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java @@ -324,18 +324,18 @@ public synchronized byte[] delete(final Bytes key) { @Override public synchronized KeyValueIterator range(final Bytes from, final Bytes to) { - return range(from, to, false); + return range(from, to, true); } @Override public synchronized KeyValueIterator reverseRange(final Bytes from, final Bytes to) { - return range(from, to, true); + return range(from, to, false); } KeyValueIterator range(final Bytes from, final Bytes to, - final boolean reverse) { + final boolean forward) { Objects.requireNonNull(from, "from cannot be null"); Objects.requireNonNull(to, "to cannot be null"); @@ -348,7 +348,7 @@ KeyValueIterator range(final Bytes from, validateStoreOpen(); - final KeyValueIterator rocksDBRangeIterator = dbAccessor.range(from, to, reverse); + final KeyValueIterator rocksDBRangeIterator = dbAccessor.range(from, to, forward); openIterators.add(rocksDBRangeIterator); return rocksDBRangeIterator; @@ -356,17 +356,17 @@ KeyValueIterator range(final Bytes from, @Override public synchronized KeyValueIterator all() { - return all(false); + return all(true); } @Override public KeyValueIterator reverseAll() { - return all(true); + return all(false); } - KeyValueIterator all(final boolean reverse) { + KeyValueIterator all(final boolean forward) { validateStoreOpen(); - final KeyValueIterator rocksDbIterator = dbAccessor.all(reverse); + final KeyValueIterator rocksDbIterator = dbAccessor.all(forward); openIterators.add(rocksDbIterator); return rocksDbIterator; } @@ -493,9 +493,9 @@ void prepareBatch(final List> entries, KeyValueIterator range(final Bytes from, final Bytes to, - final boolean reverse); + final boolean forward); - KeyValueIterator all(final boolean reverse); + KeyValueIterator all(final boolean forward); long approximateNumEntries() throws RocksDBException; @@ -560,25 +560,25 @@ public byte[] getOnly(final byte[] key) throws RocksDBException { @Override public KeyValueIterator range(final Bytes from, final Bytes to, - final boolean reverse) { + final boolean forward) { return new RocksDBRangeIterator( name, db.newIterator(columnFamily), openIterators, from, to, - reverse); + forward); } @Override - public KeyValueIterator all(final boolean reverse) { + public KeyValueIterator all(final boolean forward) { final RocksIterator innerIterWithTimestamp = db.newIterator(columnFamily); - if (reverse) { - innerIterWithTimestamp.seekToLast(); - } else { + if (forward) { innerIterWithTimestamp.seekToFirst(); + } else { + innerIterWithTimestamp.seekToLast(); } - return new RocksDbIterator(name, innerIterWithTimestamp, openIterators, reverse); + return new RocksDbIterator(name, innerIterWithTimestamp, openIterators, forward); } @Override diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java index 2b932aa53d972..e16577513395e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java @@ -194,31 +194,28 @@ public byte[] getOnly(final byte[] key) throws RocksDBException { @Override public KeyValueIterator range(final Bytes from, final Bytes to, - final boolean reverse) { + final boolean forward) { return new RocksDBDualCFRangeIterator( name, db.newIterator(newColumnFamily), db.newIterator(oldColumnFamily), from, to, - reverse); + forward); } @Override - public KeyValueIterator all(final boolean reverse) { + public KeyValueIterator all(final boolean forward) { final RocksIterator innerIterWithTimestamp = db.newIterator(newColumnFamily); - if (reverse) { - innerIterWithTimestamp.seekToLast(); - } else { - innerIterWithTimestamp.seekToFirst(); - } final RocksIterator innerIterNoTimestamp = db.newIterator(oldColumnFamily); - if (reverse) { - innerIterNoTimestamp.seekToLast(); - } else { + if (forward) { + innerIterWithTimestamp.seekToFirst(); innerIterNoTimestamp.seekToFirst(); + } else { + innerIterWithTimestamp.seekToLast(); + innerIterNoTimestamp.seekToLast(); } - return new RocksDBDualCFIterator(name, innerIterWithTimestamp, innerIterNoTimestamp, reverse); + return new RocksDBDualCFIterator(name, innerIterWithTimestamp, innerIterNoTimestamp, forward); } @Override @@ -272,7 +269,7 @@ private class RocksDBDualCFIterator extends AbstractIterator makeNext() { } else { next = KeyValue.pair(new Bytes(nextWithTimestamp), iterWithTimestamp.value()); nextWithTimestamp = null; - if (reverse) { - iterWithTimestamp.prev(); - } else { + if (forward) { iterWithTimestamp.next(); + } else { + iterWithTimestamp.prev(); } } } else { if (nextWithTimestamp == null) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; - if (reverse) { - iterNoTimestamp.prev(); - } else { + if (forward) { iterNoTimestamp.next(); + } else { + iterNoTimestamp.prev(); } } else { - if (!reverse) { + if (forward) { if (comparator.compare(nextNoTimestamp, nextWithTimestamp) <= 0) { next = KeyValue.pair(new Bytes(nextNoTimestamp), convertToTimestampedFormat(iterNoTimestamp.value())); nextNoTimestamp = null; @@ -356,7 +353,6 @@ public KeyValue makeNext() { iterWithTimestamp.prev(); } } - } } return next; @@ -385,30 +381,30 @@ private class RocksDBDualCFRangeIterator extends RocksDBDualCFIterator { // safe to just force lexicographic comparator here for now. private final Comparator comparator = Bytes.BYTES_LEXICO_COMPARATOR; private final byte[] rawLastKey; - private final boolean reverse; + private final boolean forward; RocksDBDualCFRangeIterator(final String storeName, final RocksIterator iterWithTimestamp, final RocksIterator iterNoTimestamp, final Bytes from, final Bytes to, - final boolean reverse) { - super(storeName, iterWithTimestamp, iterNoTimestamp, reverse); - this.reverse = reverse; - if (reverse) { - iterWithTimestamp.seekForPrev(to.get()); - iterNoTimestamp.seekForPrev(to.get()); - rawLastKey = from.get(); - if (rawLastKey == null) { - throw new NullPointerException("RocksDBDualCFRangeIterator: rawLastKey is null for key " + from); - } - } else { + final boolean forward) { + super(storeName, iterWithTimestamp, iterNoTimestamp, forward); + this.forward = forward; + if (forward) { iterWithTimestamp.seek(from.get()); iterNoTimestamp.seek(from.get()); rawLastKey = to.get(); if (rawLastKey == null) { throw new NullPointerException("RocksDBDualCFRangeIterator: rawLastKey is null for key " + to); } + } else { + iterWithTimestamp.seekForPrev(to.get()); + iterNoTimestamp.seekForPrev(to.get()); + rawLastKey = from.get(); + if (rawLastKey == null) { + throw new NullPointerException("RocksDBDualCFRangeIterator: rawLastKey is null for key " + from); + } } } @@ -419,7 +415,7 @@ public KeyValue makeNext() { if (next == null) { return allDone(); } else { - if (!reverse) { + if (forward) { if (comparator.compare(next.key.get(), rawLastKey) <= 0) { return next; } else { diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java index 5fe35e8c5ccf4..388195a81b89e 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDbIterator.java @@ -41,11 +41,11 @@ class RocksDbIterator extends AbstractIterator> implemen RocksDbIterator(final String storeName, final RocksIterator iter, final Set> openIterators, - final boolean reverse) { + final boolean forward) { this.storeName = storeName; this.iter = iter; this.openIterators = openIterators; - this.advanceIterator = reverse ? RocksIterator::prev : RocksIterator::next; + this.advanceIterator = forward ? RocksIterator::next : RocksIterator::prev; } @Override diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java index 4fdca17da4373..b8075d49c74d3 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/MergedSortedCacheKeyValueBytesStoreIteratorTest.java @@ -54,7 +54,7 @@ public void shouldIterateOverRange() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.range(namespace, from, to); final MergedSortedCacheKeyValueBytesStoreIterator iterator = - new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 2; @@ -82,7 +82,7 @@ public void shouldReverseIterateOverRange() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.reverseRange(namespace, from, to); final MergedSortedCacheKeyValueBytesStoreIterator iterator = - new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 9; @@ -185,7 +185,7 @@ public void shouldPeekNextKey() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.range(namespace, from, to); final MergedSortedCacheKeyValueBytesStoreIterator iterator = - new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 2; @@ -214,7 +214,7 @@ public void shouldPeekNextKeyReverse() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.reverseRange(namespace, from, to); final MergedSortedCacheKeyValueBytesStoreIterator iterator = - new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); + new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); final byte[][] values = new byte[8][]; int index = 0; int bytesIndex = 9; @@ -230,6 +230,6 @@ public void shouldPeekNextKeyReverse() { private MergedSortedCacheKeyValueBytesStoreIterator createIterator() { final ThreadCache.MemoryLRUCacheBytesIterator cacheIterator = cache.all(namespace); final KeyValueIterator storeIterator = new DelegatingPeekingKeyValueIterator<>("store", store.all()); - return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, false); + return new MergedSortedCacheKeyValueBytesStoreIterator(cacheIterator, storeIterator, true); } } \ No newline at end of file From 9a0adeb57ccb7c8c9a6174010e3a3c4bcffdec08 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Thu, 20 Aug 2020 10:28:13 +0100 Subject: [PATCH 12/14] improve range wrong order warning --- .../AbstractRocksDBSegmentedBytesStore.java | 3 ++- .../state/internals/CachingKeyValueStore.java | 6 ++++-- .../streams/state/internals/CachingSessionStore.java | 3 ++- .../streams/state/internals/CachingWindowStore.java | 3 ++- .../state/internals/InMemoryKeyValueStore.java | 3 ++- .../state/internals/InMemorySessionStore.java | 3 ++- .../streams/state/internals/InMemoryWindowStore.java | 3 ++- .../state/internals/MemoryNavigableLRUCache.java | 6 ++++-- .../kafka/streams/state/internals/RocksDBStore.java | 3 ++- .../state/internals/AbstractKeyValueStoreTest.java | 12 ++++++++---- .../internals/AbstractSessionBytesStoreTest.java | 3 ++- .../internals/AbstractWindowBytesStoreTest.java | 3 ++- .../state/internals/CachingSessionStoreTest.java | 3 ++- .../state/internals/CachingWindowStoreTest.java | 3 ++- 14 files changed, 38 insertions(+), 19 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java index dc1ae86ab08c8..ad9d3d28323f9 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java @@ -82,7 +82,8 @@ public KeyValueIterator fetch(final Bytes keyFrom, final long to) { if (keyFrom.compareTo(keyTo) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java index fe818ac0ff102..4e0a7f26a189c 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java @@ -240,7 +240,8 @@ public KeyValueIterator range(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } @@ -256,7 +257,8 @@ public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java index e7ad9688f8d80..93c170a3ee734 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java @@ -175,7 +175,8 @@ public KeyValueIterator, byte[]> findSessions(final Bytes keyFro final long latestSessionStartTime) { if (keyFrom.compareTo(keyTo) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java index 7df0a4b7d0975..c94a8b92dc979 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java @@ -223,7 +223,8 @@ public KeyValueIterator, byte[]> fetch(final Bytes from, final long timeTo) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index f219e4692991b..8d4e8345dacf9 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -122,7 +122,8 @@ public KeyValueIterator reverseRange(final Bytes from, final Byte private KeyValueIterator range(final Bytes from, final Bytes to, final boolean forward) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java index b10f4c4b52406..a897d91911299 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java @@ -182,7 +182,8 @@ public KeyValueIterator, byte[]> findSessions(final Bytes keyFro if (keyFrom.compareTo(keyTo) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java index e7220e80178d4..3b191b3293a03 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java @@ -192,7 +192,8 @@ public KeyValueIterator, byte[]> fetch(final Bytes from, if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java index 7ce44206d9979..8c23db1efa520 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java @@ -38,7 +38,8 @@ public MemoryNavigableLRUCache(final String name, final int maxCacheSize) { public KeyValueIterator range(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } @@ -53,7 +54,8 @@ public KeyValueIterator range(final Bytes from, final Bytes to) { public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java index 73649f566e35a..f6c812ebf2378 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java @@ -341,7 +341,8 @@ KeyValueIterator range(final Bytes from, if (from.compareTo(to) > 0) { log.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + + + "This may be due to range arguments set in the wrong order, " + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java index 7345f8694083e..4b777f38205c6 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java @@ -499,7 +499,8 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -515,7 +516,8 @@ public void shouldNotThrowInvalidReverseRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -531,7 +533,8 @@ public void shouldNotThrowInvalidRangeExceptionWithFromLargerThanTo() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -547,7 +550,8 @@ public void shouldNotThrowInvalidReverseRangeExceptionWithFromLargerThanTo() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java index 3147c80f3f012..bbe113158ee2c 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java @@ -554,7 +554,8 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java index 2211bd3dc200d..507e55813ec96 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java @@ -816,7 +816,8 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java index 5283c38e7eebd..a2978efd4458c 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java @@ -532,7 +532,8 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java index 097ebe9a58953..3c7af9762d211 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java @@ -623,7 +623,8 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { assertThat( messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + - " This may be due to serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + " This may be due to range arguments set in the wrong order, " + +"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } From 4dd41696e7d9855f77603c13b53648257981bfd0 Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Thu, 20 Aug 2020 16:49:04 +0100 Subject: [PATCH 13/14] fix style --- .../internals/AbstractKeyValueStoreTest.java | 8 ++--- .../AbstractSessionBytesStoreTest.java | 16 ++++----- .../AbstractWindowBytesStoreTest.java | 34 +++++++++---------- .../internals/CachingSessionStoreTest.java | 2 +- .../internals/CachingWindowStoreTest.java | 5 +-- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java index 4b777f38205c6..61f317d495416 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractKeyValueStoreTest.java @@ -500,7 +500,7 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -517,7 +517,7 @@ public void shouldNotThrowInvalidReverseRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -534,7 +534,7 @@ public void shouldNotThrowInvalidRangeExceptionWithFromLargerThanTo() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -551,7 +551,7 @@ public void shouldNotThrowInvalidReverseRangeExceptionWithFromLargerThanTo() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java index bbe113158ee2c..ce3aa86fd2965 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractSessionBytesStoreTest.java @@ -273,25 +273,25 @@ public void shouldFetchExactKeys() { new SessionWindow(0x7a00000000000000L - 2, 0x7a00000000000000L - 1)), 5L); try (final KeyValueIterator, Long> iterator = - sessionStore.findSessions("a", 0, Long.MAX_VALUE) + sessionStore.findSessions("a", 0, Long.MAX_VALUE) ) { assertThat(valuesToSet(iterator), equalTo(new HashSet<>(asList(1L, 3L, 5L)))); } try (final KeyValueIterator, Long> iterator = - sessionStore.findSessions("aa", 0, Long.MAX_VALUE) + sessionStore.findSessions("aa", 0, Long.MAX_VALUE) ) { assertThat(valuesToSet(iterator), equalTo(new HashSet<>(asList(2L, 4L)))); } try (final KeyValueIterator, Long> iterator = - sessionStore.findSessions("a", "aa", 0, Long.MAX_VALUE) + sessionStore.findSessions("a", "aa", 0, Long.MAX_VALUE) ) { assertThat(valuesToSet(iterator), equalTo(new HashSet<>(asList(1L, 2L, 3L, 4L, 5L)))); } try (final KeyValueIterator, Long> iterator = - sessionStore.findSessions("a", "aa", 10, 0) + sessionStore.findSessions("a", "aa", 10, 0) ) { assertThat(valuesToSet(iterator), equalTo(new HashSet<>(Collections.singletonList(2L)))); } @@ -304,9 +304,9 @@ public void shouldFetchAndIterateOverExactBinaryKeys() { sessionStore.init(context, sessionStore); - final Bytes key1 = Bytes.wrap(new byte[]{0}); - final Bytes key2 = Bytes.wrap(new byte[]{0, 0}); - final Bytes key3 = Bytes.wrap(new byte[]{0, 0, 0}); + final Bytes key1 = Bytes.wrap(new byte[] {0}); + final Bytes key2 = Bytes.wrap(new byte[] {0, 0}); + final Bytes key3 = Bytes.wrap(new byte[] {0, 0, 0}); sessionStore.put(new Windowed<>(key1, new SessionWindow(1, 100)), "1"); sessionStore.put(new Windowed<>(key2, new SessionWindow(2, 100)), "2"); @@ -555,7 +555,7 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java index 507e55813ec96..a40621697c33a 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractWindowBytesStoreTest.java @@ -668,10 +668,10 @@ public void shouldFetchAndIterateOverExactKeys() { final long windowSize = 0x7a00000000000000L; final long retentionPeriod = 0x7a00000000000000L; final WindowStore windowStore = buildWindowStore(retentionPeriod, - windowSize, - false, - Serdes.String(), - Serdes.String()); + windowSize, + false, + Serdes.String(), + Serdes.String()); windowStore.init(context, windowStore); @@ -754,15 +754,15 @@ public void shouldThrowNullPointerExceptionOnRangeNullToKey() { @Test public void shouldFetchAndIterateOverExactBinaryKeys() { final WindowStore windowStore = buildWindowStore(RETENTION_PERIOD, - WINDOW_SIZE, - true, - Serdes.Bytes(), - Serdes.String()); + WINDOW_SIZE, + true, + Serdes.Bytes(), + Serdes.String()); windowStore.init(context, windowStore); - final Bytes key1 = Bytes.wrap(new byte[]{0}); - final Bytes key2 = Bytes.wrap(new byte[]{0, 0}); - final Bytes key3 = Bytes.wrap(new byte[]{0, 0, 0}); + final Bytes key1 = Bytes.wrap(new byte[] {0}); + final Bytes key2 = Bytes.wrap(new byte[] {0, 0}); + final Bytes key3 = Bytes.wrap(new byte[] {0, 0, 0}); windowStore.put(key1, "1", 0); windowStore.put(key2, "2", 0); windowStore.put(key3, "3", 0); @@ -817,7 +817,7 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } @@ -1016,8 +1016,8 @@ public void testFetchDuplicates() { @SuppressWarnings("deprecation") private void putFirstBatch(final WindowStore store, - @SuppressWarnings("SameParameterValue") final long startTime, - final InternalMockProcessorContext context) { + @SuppressWarnings("SameParameterValue") final long startTime, + final InternalMockProcessorContext context) { context.setRecordContext(createRecordContext(startTime)); store.put(0, "zero"); context.setRecordContext(createRecordContext(startTime + 1L)); @@ -1032,8 +1032,8 @@ private void putFirstBatch(final WindowStore store, @SuppressWarnings("deprecation") private void putSecondBatch(final WindowStore store, - @SuppressWarnings("SameParameterValue") final long startTime, - final InternalMockProcessorContext context) { + @SuppressWarnings("SameParameterValue") final long startTime, + final InternalMockProcessorContext context) { context.setRecordContext(createRecordContext(startTime + 3L)); store.put(2, "two+1"); context.setRecordContext(createRecordContext(startTime + 4L)); @@ -1049,7 +1049,7 @@ private void putSecondBatch(final WindowStore store, } private Map> entriesByKey(final List> changeLog, - @SuppressWarnings("SameParameterValue") final long startTime) { + @SuppressWarnings("SameParameterValue") final long startTime) { final HashMap> entriesByKey = new HashMap<>(); for (final KeyValue entry : changeLog) { diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java index a2978efd4458c..d8e97b8b72f29 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingSessionStoreTest.java @@ -533,7 +533,7 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } diff --git a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java index 3c7af9762d211..49a3a95a910b9 100644 --- a/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java +++ b/streams/src/test/java/org/apache/kafka/streams/state/internals/CachingWindowStoreTest.java @@ -169,7 +169,8 @@ public KeyValue transform(final String key, final String value) } @Override - public void close() {} + public void close() { + } }, "store-name"); final String bootstrapServers = "localhost:9092"; @@ -624,7 +625,7 @@ public void shouldNotThrowInvalidRangeExceptionWithNegativeFromKey() { messages, hasItem("Returning empty iterator for fetch with invalid key range: from > to." + " This may be due to range arguments set in the wrong order, " + -"or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes." + " Note that the built-in numerical serdes do not follow this for negative numbers") ); } From 38c3bb5e35a11c4e553c896b685ca141c960dbed Mon Sep 17 00:00:00 2001 From: Jorge Esteban Quilcate Otoya Date: Thu, 20 Aug 2020 17:39:08 +0100 Subject: [PATCH 14/14] fix concat style --- .../internals/AbstractRocksDBSegmentedBytesStore.java | 4 ++-- .../streams/state/internals/CachingKeyValueStore.java | 8 ++++---- .../streams/state/internals/CachingSessionStore.java | 4 ++-- .../kafka/streams/state/internals/CachingWindowStore.java | 4 ++-- .../streams/state/internals/InMemoryKeyValueStore.java | 4 ++-- .../streams/state/internals/InMemorySessionStore.java | 4 ++-- .../streams/state/internals/InMemoryWindowStore.java | 4 ++-- .../streams/state/internals/MemoryNavigableLRUCache.java | 8 ++++---- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java index ad9d3d28323f9..39335967aaef2 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSegmentedBytesStore.java @@ -81,8 +81,8 @@ public KeyValueIterator fetch(final Bytes keyFrom, final long from, final long to) { if (keyFrom.compareTo(keyTo) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java index 4e0a7f26a189c..cae38e0d68078 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingKeyValueStore.java @@ -239,8 +239,8 @@ private byte[] getInternal(final Bytes key) { public KeyValueIterator range(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); @@ -256,8 +256,8 @@ public KeyValueIterator range(final Bytes from, public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java index 93c170a3ee734..4ac43a216c3fe 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingSessionStore.java @@ -174,8 +174,8 @@ public KeyValueIterator, byte[]> findSessions(final Bytes keyFro final long earliestSessionEndTime, final long latestSessionStartTime) { if (keyFrom.compareTo(keyTo) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java index c94a8b92dc979..8867602c45771 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/CachingWindowStore.java @@ -222,8 +222,8 @@ public KeyValueIterator, byte[]> fetch(final Bytes from, final long timeFrom, final long timeTo) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java index 8d4e8345dacf9..b02459dc57a67 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryKeyValueStore.java @@ -121,8 +121,8 @@ public KeyValueIterator reverseRange(final Bytes from, final Byte private KeyValueIterator range(final Bytes from, final Bytes to, final boolean forward) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java index a897d91911299..e4fda06682c86 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemorySessionStore.java @@ -181,8 +181,8 @@ public KeyValueIterator, byte[]> findSessions(final Bytes keyFro removeExpiredSegments(); if (keyFrom.compareTo(keyTo) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java index 3b191b3293a03..14d33cb4ea47b 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/InMemoryWindowStore.java @@ -191,8 +191,8 @@ public KeyValueIterator, byte[]> fetch(final Bytes from, removeExpiredSegments(); if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); diff --git a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java index 8c23db1efa520..93adc25520cf8 100644 --- a/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java +++ b/streams/src/main/java/org/apache/kafka/streams/state/internals/MemoryNavigableLRUCache.java @@ -37,8 +37,8 @@ public MemoryNavigableLRUCache(final String name, final int maxCacheSize) { @Override public KeyValueIterator range(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator(); @@ -53,8 +53,8 @@ public KeyValueIterator range(final Bytes from, final Bytes to) { @Override public KeyValueIterator reverseRange(final Bytes from, final Bytes to) { if (from.compareTo(to) > 0) { - LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " - + "This may be due to range arguments set in the wrong order, " + + LOG.warn("Returning empty iterator for fetch with invalid key range: from > to. " + + "This may be due to range arguments set in the wrong order, " + "or serdes that don't preserve ordering when lexicographically comparing the serialized bytes. " + "Note that the built-in numerical serdes do not follow this for negative numbers"); return KeyValueIterators.emptyIterator();