Like Damian I'd also favor a proper type (KafkaStreamsMetadata) rather than
a Map-based construct.

-Michael

On Tue, Jul 12, 2016 at 8:45 PM, Damian Guy <damian....@gmail.com> wrote:

> One more thing on the above, the methods on KafkaStreams should be changed
> to something like:
>
> Collection<KafkaStreamsMetadata> allMetadata()
>
> Collection<KafkaStreamsMetadata> allMetadataForStore(final String
> storeName)
>
> KafkaStreamsMetadata metadataWithKey(final String storeName,
>                                                 final K key,
>                                                 final Serializer<K>
> keySerializer)
>
>
> Thanks,
> Damian
>
> On Tue, 12 Jul 2016 at 11:14 Damian Guy <damian....@gmail.com> wrote:
>
> > Hi,
> >
> > I agree with point 1. application.server is a better name for the config
> > (we'll change this). However, on point 2 I think we should stick mostly
> > with what we already have. I've tried both ways of doing this when
> working
> > on the JIRA and building examples and I find the current approach more
> > intuitive and easier to use than the Map based approach.
> > However, there is probably a naming issue. We should rename
> > KafkaStreamsInstance to KafkaStreamsMetadata. This Class is very simple,
> > but provides all the information a developer needs to be able to find the
> > instance(s) of a Streams application that a particular store is running
> on,
> > i.e.,
> >
> > public class KafkStreamsMetadata {
> >     private final HostInfo hostInfo;
> >     private final Set<String> stateStoreNames;
> >     private final Set<TopicPartition> topicPartitions;
> >
> >
> > So using the API to route to a new host is fairly simple, particularly in
> > the case when you want to find the host for a particular key, i.e.,
> >
> > final KafkaStreams kafkaStreams = createKafkaStreams();
> > final KafkaStreamsMetadata streamsMetadata =
> kafkaStreams.instanceWithKey("word-count", "hello",
> Serdes.String().serializer());
> > http.get("http://"; + streamsMetadata.host() + ":" +
> streamsMetadata.port() + "/get/word-count/hello");
> >
> >
> > And if you want to do a scatter gather approach:
> >
> > final KafkaStreams kafkaStreams = createKafkaStreams();
> > final Collection<KafkaStreamsMetadata> kafkaStreamsMetadatas =
> kafkaStreams.allInstancesWithStore("word-count");
> > for (KafkaStreamsMetadata streamsMetadata : kafkaStreamsMetadatas) {
> >     http.get("http://"; + streamsMetadata.host() + ":" +
> streamsMetadata.port() + "/get/word-count/hello");
> >     ...
> > }
> >
> >
> > And if you iterated over all instances:
> >
> > final KafkaStreams kafkaStreams = createKafkaStreams();
> > final Collection<KafkaStreamsMetadata> kafkaStreamsMetadatas =
> kafkaStreams.allInstances();
> > for (KafkaStreamsMetadata streamsMetadata : kafkaStreamsMetadatas) {
> >     if (streamsMetadata.stateStoreNames().contains("word-count")) {
> >         http.get("http://"; + streamsMetadata.host() + ":" +
> streamsMetadata.port() + "/get/word-count/hello");
> >         ...
> >     }
> > }
> >
> >
> > If we were to change this to use Map<HostInfo, Set<TaskMetadata>> for the
> > most part users would need to iterate over the entry or key set.
> Examples:
> >
> > The finding an instance by key is a little odd:
> >
> > final KafkaStreams kafkaStreams = createKafkaStreams();
> > final Map<HostInfo, Set<TaskMetadata>> streamsMetadata =
> kafkaStreams.instanceWithKey("word-count","hello",
> Serdes.String().serializer());
> > // this is a bit odd as i only expect one:
> > for (HostInfo hostInfo : streamsMetadata.keySet()) {
> >     http.get("http://"; + streamsMetadata.host() + ":" +
> streamsMetadata.port() + "/get/word-count/hello");
> > }
> >
> >
> > The scatter/gather by store is fairly similar to the previous example:
> >
> > final KafkaStreams kafkaStreams = createKafkaStreams();
> > final Map<HostInfo, Set<TaskMetadata>> streamsMetadata =
> kafkaStreams.allInstancesWithStore("word-count");
> > for(HostInfo hostInfo : streamsMetadata.keySet()) {
> >     http.get("http://"; + hostInfo.host() + ":" + hostInfo.port() +
> "/get/word-count/hello");
> >     ...
> > }
> >
> > And iterating over all instances:
> >
> > final Map<HostInfo, Set<TaskMetadata>> streamsMetadata =
> kafkaStreams.allInstances();
> > for (Map.Entry<HostInfo, Set<TaskMetadata>> entry :
> streamsMetadata.entrySet()) {
> >     for (TaskMetadata taskMetadata : entry.getValue()) {
> >         if (taskMetadata.stateStoreNames().contains("word-count")) {
> >             http.get("http://"; + streamsMetadata.host() + ":" +
> streamsMetadata.port() + "/get/word-count/hello");
> >             ...
> >         }
> >     }
> > }
> >
> >
> > IMO - having a class we return is the better approach as it nicely wraps
> > the related things, i.e, host:port, store names, topic partitions into an
> > Object that is easy to use. Further we could add some behaviour to this
> > class if we felt it necessary, i.e, hasStore(storeName) etc.
> >
> > Anyway, i'm interested in your thoughts.
> >
> > Thanks,
> > Damian
> >
> > On Mon, 11 Jul 2016 at 13:47 Guozhang Wang <wangg...@gmail.com> wrote:
> >
> >> 1. Re StreamsConfig.USER_ENDPOINT_CONFIG:
> >>
> >> I agree with Neha that Kafka Streams can provide the bare minimum APIs
> >> just
> >> for host/port, and user's implemented layer can provide URL / proxy
> >> address
> >> they want to build on top of it.
> >>
> >>
> >> 2. Re Improving KafkaStreamsInstance interface:
> >>
> >> Users are indeed aware of "TaskId" class which is not part of internal
> >> packages and is exposed in PartitionGrouper interface that can be
> >> instantiated by the users, which is assigned with input topic
> partitions.
> >> So we can probably change the APIs as:
> >>
> >> Map<HostState, Set<TaskMetadata>> KafkaStreams.getAllTasks() where
> >> TaskMetadata has fields such as taskId, list of assigned partitions,
> list
> >> of state store names; and HostState can include hostname / port. The
> port
> >> is the listening port of a user-defined listener that users provide to
> >> listen for queries (e.g., using REST APIs).
> >>
> >> Map<HostState, Set<TaskMetadata>> KafkaStreams.getTasksWithStore(String
> /*
> >> storeName */) would return only the hosts and their assigned tasks if at
> >> least one of the tasks include the given store name.
> >>
> >> Map<HostState, Set<TaskMetadata>>
> KafkaStreams.getTaskWithStoreAndKey(Key
> >> k, String /* storeName */, StreamPartitioner partitioner) would return
> >> only
> >> the host and their assigned task if the store with the store name has a
> >> particular key, according to the partitioner behavior.
> >>
> >>
> >>
> >> Guozhang
> >>
> >>
> >> On Sun, Jul 10, 2016 at 11:21 AM, Neha Narkhede <n...@confluent.io>
> >> wrote:
> >>
> >> > Few thoughts that became apparent after observing example code of what
> >> an
> >> > application architecture and code might look like with these changes.
> >> > Apologize for the late realization hence.
> >> >
> >> > 1. "user.endpoint" will be very differently defined for respective
> >> > applications. I don't think Kafka Streams should generalize to accept
> >> any
> >> > connection URL as we expect to only expose metadata expressed as
> >> HostInfo
> >> > (which is defined by host & port) and hence need to interpret the
> >> > "user.endpoint" as host & port. Applications will have their own
> >> endpoint
> >> > configs that will take many forms and they will be responsible for
> >> parsing
> >> > out host and port and configuring Kafka Streams accordingly.
> >> >
> >> > If we are in fact limiting to host and port, I wonder if we should
> >> change
> >> > the name of "user.endpoint" into something more specific. We have
> >> clients
> >> > expose host/port pairs as "bootstrap.servers". Should this be
> >> > "application.server"?
> >> >
> >> > 2. I don't think we should expose another abstraction called
> >> > KafkaStreamsInstance to the user. This is related to the discussion of
> >> the
> >> > right abstraction that we want to expose to an application. The
> >> abstraction
> >> > discussion itself should probably be part of the KIP itself, let me
> >> give a
> >> > quick summary of my thoughts here:
> >> > 1. The person implementing an application using Queryable State has
> >> likely
> >> > already made some choices for the service layer–a REST framework,
> >> Thrift,
> >> > or whatever. We don't really want to add another RPC framework to this
> >> mix,
> >> > nor do we want to try to make Kafka's RPC mechanism general purpose.
> >> > 2. Likewise, it should be clear that the API you want to expose to the
> >> > front-end/client service is not necessarily the API you'd need
> >> internally
> >> > as there may be additional filtering/processing in the router.
> >> >
> >> > Given these constraints, what we prefer to add is a fairly low-level
> >> > "toolbox" that would let you do anything you want, but requires to
> route
> >> > and perform any aggregation or processing yourself. This pattern is
> >> > not recommended for all kinds of services/apps, but there are
> >> definitely a
> >> > category of things where it is a big win and other advanced
> applications
> >> > are out-of-scope.
> >> >
> >> > The APIs we expose should take the following things into
> consideration:
> >> > 1. Make it clear to the user that they will do the routing,
> aggregation,
> >> > processing themselves. So the bare minimum that we want to expose is
> >> store
> >> > and partition metadata per application server identified by the host
> and
> >> > port.
> >> > 2. Ensure that the API exposes abstractions that are known to the user
> >> or
> >> > are intuitive to the user.
> >> > 3. Avoid exposing internal objects or implementation details to the
> >> user.
> >> >
> >> > So tying all this into answering the question of what we should expose
> >> > through the APIs -
> >> >
> >> > In Kafka Streams, the user is aware of the concept of tasks and
> >> partitions
> >> > since the application scales with the number of partitions and tasks
> are
> >> > the construct for logical parallelism. The user is also aware of the
> >> > concept of state stores though until now they were not user
> accessible.
> >> > With Queryable State, the bare minimum abstractions that we need to
> >> expose
> >> > are state stores and the location of state store partitions.
> >> >
> >> > For exposing the state stores, the getStore() APIs look good but I
> think
> >> > for locating the state store partitions, we should go back to the
> >> original
> >> > proposal of simply exposing some sort of getPartitionMetadata() that
> >> > returns a PartitionMetadata or TaskMetadata object keyed by HostInfo.
> >> >
> >> > The application will convert the HostInfo (host and port) into some
> >> > connection URL to talk to the other app instances via its own RPC
> >> mechanism
> >> > depending on whether it needs to scatter-gather or just query. The
> >> > application will know how a key maps to a partition and through
> >> > PartitionMetadata it will know how to locate the server that hosts the
> >> > store that has the partition hosting that key.
> >> >
> >> > On Fri, Jul 8, 2016 at 9:40 AM, Michael Noll <mich...@confluent.io>
> >> wrote:
> >> >
> >> > > Addendum in case my previous email wasn't clear:
> >> > >
> >> > > > So for any given instance of a streams application there will
> never
> >> be
> >> > > both a v1 and v2 alive at the same time
> >> > >
> >> > > That's right.  But the current live instance will be able to tell
> >> other
> >> > > instances, via its endpoint setting, whether it wants to be
> contacted
> >> at
> >> > v1
> >> > > or at v2.  The other instances can't guess that.  Think: if an older
> >> > > instance would manually compose the "rest" of an endpoint URI,
> having
> >> > only
> >> > > the host and port from the endpoint setting, it might not know that
> >> the
> >> > new
> >> > > instances have a different endpoint suffix, for example).
> >> > >
> >> > >
> >> > > On Fri, Jul 8, 2016 at 6:37 PM, Michael Noll <mich...@confluent.io>
> >> > wrote:
> >> > >
> >> > > > Damian,
> >> > > >
> >> > > > about the rolling upgrade comment:  An instance A will contact
> >> another
> >> > > > instance B by the latter's endpoint, right?  So if A has no
> further
> >> > > > information available than B's host and port, then how should
> >> instance
> >> > A
> >> > > > know whether it should call B at /v1/ or at /v2/?  I agree that my
> >> > > > suggestion isn't foolproof, but it is afaict better than the
> >> host:port
> >> > > > approach.
> >> > > >
> >> > > >
> >> > > >
> >> > > > On Fri, Jul 8, 2016 at 5:15 PM, Damian Guy <damian....@gmail.com>
> >> > wrote:
> >> > > >
> >> > > >> Michael - i'm ok with changing it to a string. Any one else have
> a
> >> > > strong
> >> > > >> opinion on this?
> >> > > >>
> >> > > >> FWIW - i don't think it will work fine as is during the rolling
> >> > upgrade
> >> > > >> scenario as the service that is listening on the port needs to be
> >> > > embedded
> >> > > >> within each instance. So for any given instance of a streams
> >> > application
> >> > > >> there will never be both a v1 and v2 alive at the same time
> >> (unless of
> >> > > >> course the process didn't shutdown properly, but then you have
> >> another
> >> > > >> problem...).
> >> > > >>
> >> > > >> On Fri, 8 Jul 2016 at 15:26 Michael Noll <mich...@confluent.io>
> >> > wrote:
> >> > > >>
> >> > > >> > I have one further comment about
> >> > `StreamsConfig.USER_ENDPOINT_CONFIG`.
> >> > > >> >
> >> > > >> > I think we should consider to not restricting the value of this
> >> > > setting
> >> > > >> to
> >> > > >> > only `host:port` pairs.  By design, this setting is capturing
> >> > > >> user-driven
> >> > > >> > metadata to define an endpoint, so why restrict the creativity
> or
> >> > > >> > flexibility of our users?  I can imagine, for example, that
> users
> >> > > would
> >> > > >> > like to set values such as `https://host:port/api/rest/v1/` in
> >> this
> >> > > >> field
> >> > > >> > (e.g. being able to distinguish between `.../v1/` and `.../v2/`
> >> may
> >> > > >> help in
> >> > > >> > scenarios such as rolling upgrades, where, during the upgrade,
> >> older
> >> > > >> > instances may need to coexist with newer instances).
> >> > > >> >
> >> > > >> > That said, I don't have a strong opinion here.
> >> > > >> >
> >> > > >> > -Michael
> >> > > >> >
> >> > > >> >
> >> > > >> >
> >> > > >> > On Fri, Jul 8, 2016 at 2:55 PM, Matthias J. Sax <
> >> > > matth...@confluent.io>
> >> > > >> > wrote:
> >> > > >> >
> >> > > >> > > +1
> >> > > >> > >
> >> > > >> > > On 07/08/2016 11:03 AM, Eno Thereska wrote:
> >> > > >> > > > +1 (non-binding)
> >> > > >> > > >
> >> > > >> > > >> On 7 Jul 2016, at 18:31, Sriram Subramanian <
> >> r...@confluent.io>
> >> > > >> wrote:
> >> > > >> > > >>
> >> > > >> > > >> +1
> >> > > >> > > >>
> >> > > >> > > >> On Thu, Jul 7, 2016 at 9:53 AM, Henry Cai
> >> > > >> <h...@pinterest.com.invalid
> >> > > >> > >
> >> > > >> > > >> wrote:
> >> > > >> > > >>
> >> > > >> > > >>> +1
> >> > > >> > > >>>
> >> > > >> > > >>> On Thu, Jul 7, 2016 at 6:48 AM, Michael Noll <
> >> > > >> mich...@confluent.io>
> >> > > >> > > wrote:
> >> > > >> > > >>>
> >> > > >> > > >>>> +1 (non-binding)
> >> > > >> > > >>>>
> >> > > >> > > >>>> On Thu, Jul 7, 2016 at 10:24 AM, Damian Guy <
> >> > > >> damian....@gmail.com>
> >> > > >> > > >>> wrote:
> >> > > >> > > >>>>
> >> > > >> > > >>>>> Thanks Henry - we've updated the KIP with an example
> and
> >> the
> >> > > new
> >> > > >> > > config
> >> > > >> > > >>>>> parameter required. FWIW the user doesn't register a
> >> > listener,
> >> > > >> they
> >> > > >> > > >>>> provide
> >> > > >> > > >>>>> a host:port in config. It is expected they will start a
> >> > > service
> >> > > >> > > running
> >> > > >> > > >>>> on
> >> > > >> > > >>>>> that host:port that they can use to connect to the
> >> running
> >> > > >> > > KafkaStreams
> >> > > >> > > >>>>> Instance.
> >> > > >> > > >>>>>
> >> > > >> > > >>>>> Thanks,
> >> > > >> > > >>>>> Damian
> >> > > >> > > >>>>>
> >> > > >> > > >>>>> On Thu, 7 Jul 2016 at 06:06 Henry Cai
> >> > > >> <h...@pinterest.com.invalid>
> >> > > >> > > >>>> wrote:
> >> > > >> > > >>>>>
> >> > > >> > > >>>>>> It wasn't quite clear to me how the user program
> >> interacts
> >> > > with
> >> > > >> > the
> >> > > >> > > >>>>>> discovery API, especially on the user supplied
> listener
> >> > part,
> >> > > >> how
> >> > > >> > > >>> does
> >> > > >> > > >>>>> the
> >> > > >> > > >>>>>> user program supply that listener to KafkaStreams and
> >> how
> >> > > does
> >> > > >> > > >>>>> KafkaStreams
> >> > > >> > > >>>>>> know which port the user listener is running, maybe a
> >> more
> >> > > >> > complete
> >> > > >> > > >>>>>> end-to-end example including the steps on registering
> >> the
> >> > > user
> >> > > >> > > >>> listener
> >> > > >> > > >>>>> and
> >> > > >> > > >>>>>> whether the user listener needs to be involved with
> task
> >> > > >> > > >>> reassignment.
> >> > > >> > > >>>>>>
> >> > > >> > > >>>>>>
> >> > > >> > > >>>>>> On Wed, Jul 6, 2016 at 9:13 PM, Guozhang Wang <
> >> > > >> wangg...@gmail.com
> >> > > >> > >
> >> > > >> > > >>>>> wrote:
> >> > > >> > > >>>>>>
> >> > > >> > > >>>>>>> +1
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>> On Wed, Jul 6, 2016 at 12:44 PM, Damian Guy <
> >> > > >> > damian....@gmail.com>
> >> > > >> > > >>>>>> wrote:
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>>> Hi all,
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>> I'd like to initiate the voting process for KIP-67
> >> > > >> > > >>>>>>>> <
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>
> >> > > >> > > >>>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>
> >> > > >> > >
> >> > > >> >
> >> > > >>
> >> > >
> >> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-67%3A+Queryable+state+for+Kafka+Streams
> >> > > >> > > >>>>>>>>>
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>> KAFKA-3909 <
> >> > > https://issues.apache.org/jira/browse/KAFKA-3909
> >> > > >> >
> >> > > >> > is
> >> > > >> > > >>>> the
> >> > > >> > > >>>>>> top
> >> > > >> > > >>>>>>>> level JIRA for this effort.
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>> Initial PRs for Step 1 of the process are:
> >> > > >> > > >>>>>>>> Expose State Store Names <
> >> > > >> > > >>>> https://github.com/apache/kafka/pull/1526>
> >> > > >> > > >>>>>> and
> >> > > >> > > >>>>>>>> Query Local State Stores <
> >> > > >> > > >>>> https://github.com/apache/kafka/pull/1565>
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>> Thanks,
> >> > > >> > > >>>>>>>> Damian
> >> > > >> > > >>>>>>>>
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>> --
> >> > > >> > > >>>>>>> -- Guozhang
> >> > > >> > > >>>>>>>
> >> > > >> > > >>>>>>
> >> > > >> > > >>>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>> --
> >> > > >> > > >>>> Best regards,
> >> > > >> > > >>>> Michael Noll
> >> > > >> > > >>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>>
> >> > > >> > > >>>> *Michael G. Noll | Product Manager | Confluent | +1
> >> > > >> > > 650.453.5860Download
> >> > > >> > > >>>> Apache Kafka and Confluent Platform:
> >> > www.confluent.io/download
> >> > > >> > > >>>> <http://www.confluent.io/download>*
> >> > > >> > > >>>>
> >> > > >> > > >>>
> >> > > >> > > >
> >> > > >> > >
> >> > > >> > >
> >> > > >> >
> >> > > >> >
> >> > > >> > --
> >> > > >> > Best regards,
> >> > > >> > Michael Noll
> >> > > >> >
> >> > > >> >
> >> > > >> >
> >> > > >> > *Michael G. Noll | Product Manager | Confluent | +1
> >> > > 650.453.5860Download
> >> > > >> > Apache Kafka and Confluent Platform: www.confluent.io/download
> >> > > >> > <http://www.confluent.io/download>*
> >> > > >> >
> >> > > >>
> >> > > >
> >> > > >
> >> > > >
> >> > > > --
> >> > > > Best regards,
> >> > > > Michael Noll
> >> > > >
> >> > > >
> >> > > >
> >> > > > *Michael G. Noll | Product Manager | Confluent | +1 650.453.5860
> >> > > > <%2B1%20650.453.5860>Download Apache Kafka and Confluent Platform:
> >> > > > www.confluent.io/download <http://www.confluent.io/download>*
> >> > > >
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > > Best regards,
> >> > > Michael Noll
> >> > >
> >> > >
> >> > >
> >> > > *Michael G. Noll | Product Manager | Confluent | +1
> >> 650.453.5860Download
> >> > > Apache Kafka and Confluent Platform: www.confluent.io/download
> >> > > <http://www.confluent.io/download>*
> >> > >
> >> >
> >> >
> >> >
> >> > --
> >> > Thanks,
> >> > Neha
> >> >
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
>



-- 
Best regards,
Michael Noll



*Michael G. Noll | Product Manager | Confluent | +1 650.453.5860Download
Apache Kafka and Confluent Platform: www.confluent.io/download
<http://www.confluent.io/download>*

Reply via email to