Re: Role of CompositeReadOnlyKeyValueStore for point queries

2020-01-16 Thread Navinder Brar
Thanks John for the prompt response. Yeah, store(int partition) is exactly what 
I meant. Sorry for the confusion. Since we have already merged PR:7962 I can 
take the new trunk and start working on the KIP in the meantime. I agree that 
we should replace the newly added param. It would be a good improvement for 
improving latencies for applications that contain multiple partitions on a 
single instance and don't have bloom filters enabled internally for Rocksdb.

On Thursday, 16 January, 2020, 10:22:45 pm IST, John Roesler 
 wrote:  
 
 Hey Navinder,

I agree this is not ideal. The answer is that until KIP-535, you wouldn't
necessarily know which partition you were querying, just the instance.

Now, though, the information is front-and-center, and we should
update the store() API to favor efficiently routing the query directly
to the right store instance, rather than iterating over all the stores.

I'd been privately planning to propose a new `KafkaStreams#store()`
method that lets you just request a store for a particular partition.
Reading your message, I think maybe this is what you meant, instead
of `get()`?

If you want to take it on, I'd be more than happy to take the lead on
reviewing your work.

One food for thought is that we just added a new overload:
`store(name, type, boolean includeStaleStores)`. Instead of adding
another new overload, we should replace the new overload with
one taking a config object.

Suggestion:
KafkaStreams#store(
  String storeName,
  QueryableStoreType type,
  StoreQueryParams options
)

where:
StoreQueryParams {
  (enable/disable/get)IncludeStaleStores()
  (with/get)partitions(List)
}

Although this isn't a DSL operation, please consider 
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams+DSL+Grammar
when proposing new interfaces.

If you're _really_ fast, we could potentially get this in to 2.5:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=143428858
We'd basically have to call for a vote by Friday to have a chance of meeting 
the deadline.
Then, we could just replace the new overload instead of deprecating it.
But it's not such a huge deal to deprecate it, so no pressure.

Thanks for the astute observation!
-John

On Wed, Jan 15, 2020, at 22:50, Navinder Brar wrote:
> Hi all,
> Can someone explain to me the thoughts behind having 
> CompositeReadOnlyKeyValueStore. java class while serving data via APIs 
> in Kafka Streams. It fetches the list of stores for all the running 
> tasks on the machine and then looks for a key one by one in each of the 
> stores. When we already know the key belongs to a particular 
> partition(with the help of partitioner) we can just query that 
> particular partition's store right?
> I am thinking of overloading the get() function as get(int partition) 
> and sending just the store for that single partition from 
> QueryableStoreProvider.java so that all the stores are needed to be 
> iterated through to fetch a key.
> Regards,
> Navinder
>
  

Re: Role of CompositeReadOnlyKeyValueStore for point queries

2020-01-16 Thread John Roesler
Hey Navinder,

I agree this is not ideal. The answer is that until KIP-535, you wouldn't
necessarily know which partition you were querying, just the instance.

Now, though, the information is front-and-center, and we should
update the store() API to favor efficiently routing the query directly
to the right store instance, rather than iterating over all the stores.

I'd been privately planning to propose a new `KafkaStreams#store()`
method that lets you just request a store for a particular partition.
Reading your message, I think maybe this is what you meant, instead
of `get()`?

If you want to take it on, I'd be more than happy to take the lead on
reviewing your work.

One food for thought is that we just added a new overload:
`store(name, type, boolean includeStaleStores)`. Instead of adding
another new overload, we should replace the new overload with
one taking a config object.

Suggestion:
KafkaStreams#store(
  String storeName,
  QueryableStoreType type,
  StoreQueryParams options
)

where:
StoreQueryParams {
  (enable/disable/get)IncludeStaleStores()
  (with/get)partitions(List)
}

Although this isn't a DSL operation, please consider 
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams+DSL+Grammar
when proposing new interfaces.

If you're _really_ fast, we could potentially get this in to 2.5:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=143428858
We'd basically have to call for a vote by Friday to have a chance of meeting 
the deadline.
Then, we could just replace the new overload instead of deprecating it.
But it's not such a huge deal to deprecate it, so no pressure.

Thanks for the astute observation!
-John

On Wed, Jan 15, 2020, at 22:50, Navinder Brar wrote:
> Hi all,
> Can someone explain to me the thoughts behind having 
> CompositeReadOnlyKeyValueStore. java class while serving data via APIs 
> in Kafka Streams. It fetches the list of stores for all the running 
> tasks on the machine and then looks for a key one by one in each of the 
> stores. When we already know the key belongs to a particular 
> partition(with the help of partitioner) we can just query that 
> particular partition's store right?
> I am thinking of overloading the get() function as get(int partition) 
> and sending just the store for that single partition from 
> QueryableStoreProvider.java so that all the stores are needed to be 
> iterated through to fetch a key.
> Regards,
> Navinder
>


Role of CompositeReadOnlyKeyValueStore for point queries

2020-01-15 Thread Navinder Brar
Hi all,
Can someone explain to me the thoughts behind having 
CompositeReadOnlyKeyValueStore. java class while serving data via APIs in Kafka 
Streams. It fetches the list of stores for all the running tasks on the machine 
and then looks for a key one by one in each of the stores. When we already know 
the key belongs to a particular partition(with the help of partitioner) we can 
just query that particular partition's store right?
I am thinking of overloading the get() function as get(int partition) and 
sending just the store for that single partition from 
QueryableStoreProvider.java so that all the stores are needed to be iterated 
through to fetch a key.
Regards,
Navinder