Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1169,11 +1169,9 @@ public <T> T store(final String storeName, final QueryableStoreType<T> queryable

/**
* Get a facade wrapping the local {@link StateStore} instances with the provided {@link StoreQueryParams}.
* StoreQueryParams need required parameters to be set, which are {@code storeName} and if
* type is accepted by the provided {@link QueryableStoreType#accepts(StateStore) queryableStoreType}.
* The returned object can be used to query the {@link StateStore} instances.
*
* @param storeQueryParams to set the optional parameters to fetch type of stores user wants to fetch when a key is queried
* @param storeQueryParams the parameters used to fetch a queryable store
* @return A facade wrapping the local {@link StateStore} instances
* @throws InvalidStateStoreException if Kafka Streams is (re-)initializing or a store with {@code storeName} and
* {@code queryableStoreType} doesn't exist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@
import java.util.Objects;

/**
* Represents all the query options that a user can provide to state what kind of stores it is expecting.
* The options would be whether a user would want to enable/disable stale stores
* or whether it knows the list of partitions that it specifically wants to fetch.
* If this information is not provided the default behavior is to fetch the stores for all the partitions
* available on that instance for that particular store name.
* It contains a partition, which for a point queries can be populated from the {@link KeyQueryMetadata}.
* {@code StoreQueryParams} allows you to pass a variety of parameters when fetching a store for interactive query.
*/
public class StoreQueryParams<T> {

Expand All @@ -35,46 +30,42 @@ public class StoreQueryParams<T> {
private final String storeName;
private final QueryableStoreType<T> queryableStoreType;

private StoreQueryParams(final String storeName, final QueryableStoreType<T> queryableStoreType) {
private StoreQueryParams(final String storeName, final QueryableStoreType<T> queryableStoreType, final Integer partition, final boolean staleStores) {
this.storeName = storeName;
this.queryableStoreType = queryableStoreType;
this.partition = partition;
this.staleStores = staleStores;
}

public static <T> StoreQueryParams<T> fromNameAndType(final String storeName,
final QueryableStoreType<T> queryableStoreType) {
return new StoreQueryParams<T>(storeName, queryableStoreType);
return new StoreQueryParams<T>(storeName, queryableStoreType, null, false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a different sentinel than null for the partition field?. Does an Optional make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we had a long discussion on keeping this null and adding it in Java doc that null means all partitions. Do you think this is important to start a discussion on the thread again? :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont worry about it :) .. Happy to budge on stylistic things and leave it to the committers' call

}

/**
* Set a specific partition that should be queried exclusively.
*
* @param partition The specific integer partition to be fetched from the stores list by using {@link StoreQueryParams}.
*
* @return String storeName
* @return StoreQueryParams a new {@code StoreQueryParams} instance configured with the specified partition
*/
public StoreQueryParams<T> withPartition(final Integer partition) {
final StoreQueryParams<T> storeQueryParams = StoreQueryParams.fromNameAndType(this.storeName(), this.queryableStoreType());
storeQueryParams.partition = partition;
storeQueryParams.staleStores = this.staleStores;
return storeQueryParams;
return new StoreQueryParams<T>(this.storeName(), this.queryableStoreType(), partition, this.staleStores);
Comment thread
brary marked this conversation as resolved.
Outdated
}

/**
* Enable querying of stale state stores, i.e., allow to query active tasks during restore as well as standby tasks.
*
* @return String storeName
* @return StoreQueryParams a new {@code StoreQueryParams} instance configured with serving from stale stores enabled
*/
public StoreQueryParams<T> enableStaleStores() {
final StoreQueryParams<T> storeQueryParams = StoreQueryParams.fromNameAndType(this.storeName(), this.queryableStoreType());
storeQueryParams.partition = this.partition;
storeQueryParams.staleStores = true;
return storeQueryParams;
return new StoreQueryParams<T>(this.storeName(), this.queryableStoreType(), this.partition, true);
Comment thread
brary marked this conversation as resolved.
Outdated
}

/**
* Get the store name for which key is queried by the user.
* Get the name of the state store that should be queried.
*
* @return String storeName
* @return String state store name
*/
public String storeName() {
return storeName;
Expand All @@ -83,16 +74,16 @@ public String storeName() {
/**
* Get the queryable store type for which key is queried by the user.
*
* @return QueryableStoreType queryableStoreType
* @return QueryableStoreType type of queryable store
*/
public QueryableStoreType<T> queryableStoreType() {
return queryableStoreType;
}

/**
* Get the partition to be used to fetch list of stores.
* Get the store partition that will be queried.
* If the method returns {@code null}, it would mean that no specific partition has been requested,
* so all the local partitions for the store will be returned.
* so all the local partitions for the store will be queried.
*
* @return Integer partition
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,10 @@ public Map<String, List<String>> stateStoreNameToSourceTopics() {
return results;
}

public Collection<String> sourceTopicsForStore(final String storeName) {
return maybeDecorateInternalSourceTopics(stateStoreNameToSourceTopics.get(storeName));
}

public synchronized Collection<Set<String>> copartitionGroups() {
// compute transitive closures of copartitionGroups to relieve registering code to know all members
// of a copartitionGroup at the same time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.kafka.streams.processor.internals;

import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.kafka.common.Cluster;
import org.apache.kafka.common.PartitionInfo;
Expand Down Expand Up @@ -109,7 +110,7 @@ public synchronized Collection<StreamsMetadata> getAllMetadataForStore(final Str
return allMetadata;
}

final List<String> sourceTopics = builder.stateStoreNameToSourceTopics().get(storeName);
final Collection<String> sourceTopics = builder.sourceTopicsForStore(storeName);
if (sourceTopics == null) {
return Collections.emptyList();
}
Expand Down Expand Up @@ -408,7 +409,7 @@ private <K> StreamsMetadata getStreamsMetadataForKey(final String storeName,
}

private SourceTopicsInfo getSourceTopicsInfo(final String storeName) {
final List<String> sourceTopics = builder.stateStoreNameToSourceTopics().get(storeName);
final List<String> sourceTopics = builder.sourceTopicsForStore(storeName).stream().collect(Collectors.toList());
if (sourceTopics == null || sourceTopics.isEmpty()) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Collection;
import java.util.stream.Collectors;
Comment thread
brary marked this conversation as resolved.
Outdated

public class StreamThreadStateStoreProvider {

Expand Down Expand Up @@ -91,8 +93,8 @@ private TaskId createKeyTaskId(final String storeName, final Integer partition)
if (partition == null) {
return null;
}
final List<String> sourceTopics = internalTopologyBuilder.stateStoreNameToSourceTopics().get(storeName);
final Set<String> sourceTopicsSet = new HashSet<>(sourceTopics);
final Collection<String> sourceTopics = internalTopologyBuilder.sourceTopicsForStore(storeName);
final Set<String> sourceTopicsSet = sourceTopics.stream().collect(Collectors.toSet());
final Map<Integer, InternalTopologyBuilder.TopicsInfo> topicGroups = internalTopologyBuilder.topicGroups();
for (final Map.Entry<Integer, InternalTopologyBuilder.TopicsInfo> topicGroup : topicGroups.entrySet()) {
if (topicGroup.getValue().sourceTopics.containsAll(sourceTopicsSet)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.apache.kafka.streams.StreamsBuilder;
import org.apache.kafka.streams.StreamsConfig;
import org.apache.kafka.streams.KeyValue;
import org.apache.kafka.streams.errors.InvalidStateStoreException;
import org.apache.kafka.streams.integration.utils.EmbeddedKafkaCluster;
import org.apache.kafka.streams.integration.utils.IntegrationTestUtils;
import org.apache.kafka.streams.kstream.Consumed;
Expand Down Expand Up @@ -91,7 +90,7 @@ public void after() {
}

@Test
public void shouldQueryAllActivePartitionStoresByDefault() throws Exception {
public void shouldQueryOnlyActivePartitionStoresByDefault() throws Exception {
final int batch1NumMessages = 100;
final int key = 1;
final Semaphore semaphore = new Semaphore(0);
Expand Down Expand Up @@ -164,28 +163,28 @@ public void shouldQuerySpecificActivePartitionStores() throws Exception {

//key doesn't belongs to this partition
final int keyDontBelongPartition = (keyPartition == 0) ? 1 : 0;
final boolean kafkaStreams1IsActive;
if ((keyQueryMetadata.getActiveHost().port() % 2) == 1) {
kafkaStreams1IsActive = true;
} else {
kafkaStreams1IsActive = false;
}

final QueryableStoreType<ReadOnlyKeyValueStore<Integer, Integer>> queryableStoreType = QueryableStoreTypes.keyValueStore();
ReadOnlyKeyValueStore<Integer, Integer> store1 = null;
ReadOnlyKeyValueStore<Integer, Integer> store2 = null;
try {
if (kafkaStreams1IsActive) {
store1 = kafkaStreams1
.store(StoreQueryParams.fromNameAndType(TABLE_NAME, queryableStoreType).withPartition(keyPartition));
} catch (final InvalidStateStoreException exception) {
//Only one among kafkaStreams1 and kafkaStreams2 will contain the specific active store requested. The other will throw exception
}
try {
} else {
store2 = kafkaStreams2
.store(StoreQueryParams.fromNameAndType(TABLE_NAME, queryableStoreType).withPartition(keyPartition));
} catch (final InvalidStateStoreException exception) {
//Only one among kafkaStreams1 and kafkaStreams2 will contain the specific active store requested. The other will throw exception
}
final boolean kafkaStreams1IsActive;
if ((keyQueryMetadata.getActiveHost().port() % 2) == 1) {
kafkaStreams1IsActive = true;

if (kafkaStreams1IsActive) {
assertThat(store1, is(notNullValue()));
assertThat(store2, is(nullValue()));
} else {
kafkaStreams1IsActive = false;
assertThat(store2, is(notNullValue()));
assertThat(store1, is(nullValue()));
}
Expand All @@ -195,17 +194,12 @@ public void shouldQuerySpecificActivePartitionStores() throws Exception {

ReadOnlyKeyValueStore<Integer, Integer> store3 = null;
ReadOnlyKeyValueStore<Integer, Integer> store4 = null;
try {
if (!kafkaStreams1IsActive) {
store3 = kafkaStreams1
.store(StoreQueryParams.fromNameAndType(TABLE_NAME, queryableStoreType).withPartition(keyDontBelongPartition));
} catch (final InvalidStateStoreException exception) {
//Only one among kafkaStreams1 and kafkaStreams2 will contain the specific active store requested. The other will throw exception
}
try {
} else {
store4 = kafkaStreams2
.store(StoreQueryParams.fromNameAndType(TABLE_NAME, queryableStoreType).withPartition(keyDontBelongPartition));
} catch (final InvalidStateStoreException exception) {
//Only one among kafkaStreams1 and kafkaStreams2 will contain the specific active store requested. The other will throw exception
}

// Assert that key is not served when wrong specific partition is requested
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ public void shouldMapStateStoresToCorrectSourceTopics() {
mapped.leftJoin(table, MockValueJoiner.TOSTRING_JOINER).groupByKey().count(Materialized.as("count"));
builder.buildAndOptimizeTopology();
builder.internalTopologyBuilder.rewriteTopology(new StreamsConfig(StreamsTestUtils.getStreamsConfig(APP_ID)));
assertEquals(Collections.singletonList("table-topic"), builder.internalTopologyBuilder.stateStoreNameToSourceTopics().get("table-store"));
assertEquals(Collections.singletonList(APP_ID + "-KSTREAM-MAP-0000000003-repartition"), builder.internalTopologyBuilder.stateStoreNameToSourceTopics().get("count"));
assertEquals(Collections.singletonList("table-topic"), builder.internalTopologyBuilder.sourceTopicsForStore("table-store"));
assertEquals(Collections.singletonList(APP_ID + "-KSTREAM-MAP-0000000003-repartition"), builder.internalTopologyBuilder.sourceTopicsForStore("count"));
}

@Test
Expand Down