Thanks Colin. I am familiar with the protocol semantics, but we need to document the API for users who don't know the protocol. I still think it would be valuable to have some examples of how the API would be used for common use cases. For example, say someone creates a topic and then produces to it. What would be the recommended way to do that?
Ismael On Mon, Mar 6, 2017 at 7:43 PM, Colin McCabe <cmcc...@apache.org> wrote: > On Mon, Mar 6, 2017, at 05:50, Ismael Juma wrote: > > Thanks Colin. It seems like you replied to me accidentally instead of the > > list, so leaving your reply below for the benefit of others. > > Thanks, Ismael. I actually realized my mistake right after I sent to > you, and re-posted it to the mailing list instead of sending directly. > Sigh... > > > > > Regarding the disadvantage of having to hunt through the request class, > > don't people have to do that anyway with the Options classes? > > A lot of people will simply choose the default options, until they have > a reason to do otherwise (for example, they want a longer or shorter > timeout, etc.) > > > > > Aside from that, it would be great if the KIP included more detailed > > javadoc for each method including information about potential exceptions. > > That's a good question. Because this is an asynchronous API, methods > never throw exceptions. Instead, if you call get() / whenComplete() / > isCompletedExceptionally() / etc. on one of the CompletableFuture > objects, you will get the exception. This is to allow Node.js-style > completion chaining. I will add this explanation to the KIP. > > > I'm particularly interested in what a user can expect if a create topics > > succeeds versus what they can expect if a timeout exception is thrown. It > > would be good to consider partial failures as well. > > This is spelled out by KIP-4. > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+ > Command+line+and+centralized+administrative+operations > > Specifically, > > > If a timeout error occurs [in CreateTopic], the topic could still be > > created successfully at a later time. Its up to the client to query > > for the state at that point. > > Since we're specifically not changing the server as part of this KIP, > those semantics will still be in force. Of course, there are plenty of > other exceptions that you can get from CreateTopics that are more > meaningful, such as permission-related or network-related ones. But if > you get a timeout, the operation may or may not have succeeded. > > Could we fix the timeout problem? Sort of. We could implement > something like a retry cache. The brokers would have to maintain a > cache of operations (and their results) which had succeeded or failed. > Then, if an RPC got interrupted after the server had performed it, but > before the client had received the response message, the client could > simply reconnect on another TCP session and ask the broker for the > result of the previous operation. The broker could look up the result > in the cache and re-send it. > > This fix works, but it is very complex. The cache requires space in > memory (and to do it perfectly, you also want to persist the cache to > disk in case the broker restarts and the client re-appears). The fix > also requires the client to wait for an indefinite amount of time for > the server to come back. If the client ever "gives up" and just throws > a timeout exception, we are back to not knowing what happened on the > server. > > In any case, I think we should discuss RPC change in a separate KIP... > the scope is already big enough here. Also, in practice, users have > workarounds for cases where there are timeouts or failures to > communicate. > > best, > Colin > > > > > Ismael > > > > On Fri, Mar 3, 2017 at 9:37 PM, Colin McCabe <cmcc...@apache.org> wrote: > > > > > On Fri, Mar 3, 2017, at 06:41, Ismael Juma wrote: > > > > Hi Colin, > > > > > > > > I still need to do a detailed review, but I have a couple of > > > > comments/questions: > > > > > > > > 1. I am not sure about having the options/response classes as inner > > > > classes > > > > of the interface. It means that file containing the interface will be > > > > huge > > > > eventually. And the classes are not necessarily related either. Why > not > > > > use > > > > a separate package for them? > > > > > > Yeah, I think it's reasonable to make these top-level classes and put > > > them in separate files. We can put them all in > > > org.apache.kafka.clients.admin. > > > > > > > > > > > 2. Can you elaborate on how one decides one goes in the Options class > > > > versus the first parameter? > > > > > > I guess I think of options as things that you don't have to set. For > > > example, when deleting a topic, you must supply the topic name, but > > > supplying a non-default timeout is optional. > > > > > > > I wonder if it would be simpler to just have a > > > > single parameter. In that case it should probably be called a > Request as > > > > Radai suggested, but that's a separate point and we can discuss it > > > > separately. > > > > > > Hmm. I don't think it would be simpler for users. It would force > > > people who just want to do something simple like delete a topic or get > > > the api version of a single node to go hunting through the request > > > class. > > > > > > best, > > > Colin > > > > > > > > > > > > > > Ismael > > > > > > > > On Thu, Mar 2, 2017 at 1:58 AM, Colin McCabe <cmcc...@apache.org> > wrote: > > > > > > > > > On Wed, Mar 1, 2017, at 15:52, radai wrote: > > > > > > quick comment on the request objects: > > > > > > > > > > > > i see "abstract class NewTopic" and "class > NewTopicWithReplication" > > > and " > > > > > > NewTopicWithReplicaAssignments" > > > > > > > > > > > > 1. since the result object is called CreateTopicResults should > these > > > be > > > > > > called *Request? > > > > > > > > > > Hi radai, > > > > > > > > > > Thanks for taking a look. > > > > > > > > > > I think using the name "request" would be very confusing here, > because > > > > > we have a whole family of internal Request classes such as > > > > > CreateTopicsRequest, TopicMetataRequest, etc. which are used for > RPCs. > > > > > > > > > > > 2. this seems like a suboptimal approach to me. imagine we add a > > > > > > NewTopicWithSecurity, and then we would need > > > > > > NewTopicWithReplicationAndSecurity? (or any composable > "traits"). > > > > > > this wont really scale. Wouldnt it be better to have a single > (rather > > > > > complicated) > > > > > > CreateTopicRequest, and use a builder pattern to deal with the > > > compexity > > > > > > and options? like so: > > > > > > > > > > > > CreateTopicRequest req = > > > > > > AdminRequests.newTopic("bob").replicationFactor(2). > > > > > withPartitionAssignment(1, > > > > > > "boker7", "broker10").withOption(...).build(); > > > > > > > > > > > > the builder would validate any potentially conflicting options > and > > > would > > > > > > allow piling on the complexity in a more manageable way (note - > my > > > code > > > > > > above intends to demonstrate both a general replication factor > and a > > > > > > specific assignment for a partiocular partition of that topic, > which > > > may > > > > > > be > > > > > > too much freedom). > > > > > > > > > > We don't need to express every optional bell and whistle by > creating a > > > > > subclass. In fact, the proposal already had setConfigs() in the > base > > > > > class, since it applies to every new topic creation. > > > > > > > > > > Thinking about it a little more, though, the subclasses don't > really > > > add > > > > > that much value, so we should probably just have NewTopic and no > > > > > subclasses. I removed the subclasses. > > > > > > > > > > best, > > > > > Colin > > > > > > > > > > > > > > > > > On Wed, Mar 1, 2017 at 1:58 PM, Colin McCabe <cmcc...@apache.org > > > > > wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > Thanks for commenting, everyone. Does anyone have more > questions > > > or > > > > > > > comments, or should we vote? The latest proposal is up at > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > 117%3A+Add+a+public+ > > > > > > > AdminClient+API+for+Kafka+admin+operations > > > > > > > > > > > > > > best, > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > On Thu, Feb 16, 2017, at 15:00, Colin McCabe wrote: > > > > > > > > On Thu, Feb 16, 2017, at 14:11, Dong Lin wrote: > > > > > > > > > Hey Colin, > > > > > > > > > > > > > > > > > > Thanks for the update. I have two comments: > > > > > > > > > > > > > > > > > > - I actually think it is simpler and good enough to have > > > per-topic > > > > > API > > > > > > > > > instead of batch-of-topic API. This is different from the > > > argument > > > > > for > > > > > > > > > batch-of-partition API because, unlike operation on topic, > > > people > > > > > > > usually > > > > > > > > > operate on multiple partitions (e.g. seek(), purge()) at a > > > time. Is > > > > > > > there > > > > > > > > > performance concern with per-topic API? I am wondering if > we > > > > > should do > > > > > > > > > per-topic API until there is use-case or performance > benefits > > > of > > > > > > > > > batch-of-topic API. > > > > > > > > > > > > > > > > Yes, there is a performance concern with only supporting > > > operations > > > > > on > > > > > > > > one topic at a time. Jay expressed this in some of his > earlier > > > > > emails > > > > > > > > and some other people did as well. We have cases in mind for > > > > > management > > > > > > > > software where many topics are created at once. > > > > > > > > > > > > > > > > > > > > > > > > > > - Currently we have interface "Consumer" and "Producer". > And we > > > > > also > > > > > > > have > > > > > > > > > implementations of these two interfaces as "KafkaConsumer" > and > > > > > > > > > "KafkaProducer". If we follow the same naming pattern, > should > > > we > > > > > have > > > > > > > > > interface "AdminClient" and the implementation > > > "KafkaAdminClient", > > > > > > > > > instead > > > > > > > > > of the other way around? > > > > > > > > > > > > > > > > That's a good point. We should do that for consistency. > > > > > > > > > > > > > > > > best, > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > Dong > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Feb 16, 2017 at 10:19 AM, Colin McCabe < > > > cmcc...@apache.org > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > So I think people have made some very good points so far. > > > There > > > > > > > seems > > > > > > > > > > to be agreement that we need to have explicit batch APIs > for > > > the > > > > > > > sake of > > > > > > > > > > efficiency, so I added that back. > > > > > > > > > > > > > > > > > > > > Contexts seem a little more complex than we thought, so I > > > removed > > > > > > > that > > > > > > > > > > from the proposal. > > > > > > > > > > > > > > > > > > > > I removed the Impl class. Instead, we now have a > > > > > KafkaAdminClient > > > > > > > > > > interface and an AdminClient implementation. I think > this > > > > > matches > > > > > > > our > > > > > > > > > > other code better, as Jay commented. > > > > > > > > > > > > > > > > > > > > Each call now has an "Options" object that is passed in. > > > This > > > > > will > > > > > > > > > > allow us to easily add new parameters to the calls > without > > > having > > > > > > > tons > > > > > > > > > > of function overloads. Similarly, each call now has a > > > Results > > > > > > > object, > > > > > > > > > > which will let us easily extend the results we are > returning > > > if > > > > > > > needed. > > > > > > > > > > > > > > > > > > > > Many people made the point that Java 7 Futures are not > that > > > > > useful, > > > > > > > but > > > > > > > > > > Java 8 CompletableFutures are. With CompletableFutures, > you > > > can > > > > > > > chain > > > > > > > > > > calls, adapt them, join them-- basically all the stuff > > > people are > > > > > > > doing > > > > > > > > > > in Node.js and Twisted Python. Java 7 Futures don't > really > > > let > > > > > you > > > > > > > do > > > > > > > > > > anything but poll for a value or block. So I felt that > it > > > was > > > > > > > better to > > > > > > > > > > just go with a CompletableFuture-based API. > > > > > > > > > > > > > > > > > > > > People also made the point that they would like an easy > API > > > for > > > > > > > waiting > > > > > > > > > > on complete success of a batch call. For example, an API > > > that > > > > > would > > > > > > > > > > fail if even one topic wasn't created in createTopics. > So I > > > > > came up > > > > > > > > > > with Result objects that provide multiple futures that > you > > > can > > > > > wait > > > > > > > on. > > > > > > > > > > You can wait on a future that fires when everything is > > > complete, > > > > > or > > > > > > > you > > > > > > > > > > can wait on futures for individual topics being created. > > > > > > > > > > > > > > > > > > > > I updated the wiki, so please take a look. Note that > this > > > new > > > > > API > > > > > > > > > > requires JDK8. It seems like JDK8 is coming soon, > though, > > > and > > > > > the > > > > > > > > > > disadvantages of sticking to Java 7 are pretty big here, > I > > > think. > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Feb 13, 2017, at 11:51, Colin McCabe wrote: > > > > > > > > > > > On Sun, Feb 12, 2017, at 09:21, Jay Kreps wrote: > > > > > > > > > > > > Hey Colin, > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the hard work on this. I know going back > and > > > > > forth on > > > > > > > APIs > > > > > > > > > > is > > > > > > > > > > > > kind of frustrating but we're at the point where > these > > > things > > > > > > > live long > > > > > > > > > > > > enough and are used by enough people that it is > worth the > > > > > pain. > > > > > > > I'm > > > > > > > > > > sure > > > > > > > > > > > > it'll come down in the right place eventually. A > couple > > > > > things > > > > > > > I've > > > > > > > > > > found > > > > > > > > > > > > helped in the past: > > > > > > > > > > > > > > > > > > > > > > > > 1. The burden of evidence needs to fall on the > > > > > complicator. > > > > > > > i.e. if > > > > > > > > > > > > person X thinks the api should be async they need > to > > > > > produce > > > > > > > a set > > > > > > > > > > of > > > > > > > > > > > > common use cases that require this. Otherwise you > are > > > > > > > perpetually > > > > > > > > > > > > having to > > > > > > > > > > > > think "we might need x". I think it is good to > have a > > > > > rule of > > > > > > > > > > "simple > > > > > > > > > > > > until > > > > > > > > > > > > proven insufficient". > > > > > > > > > > > > 2. Make sure we frame things for the intended > > > audience. At > > > > > > > this > > > > > > > > > > point > > > > > > > > > > > > our apis get used by a very broad set of Java > > > engineers. > > > > > This > > > > > > > is a > > > > > > > > > > > > very > > > > > > > > > > > > different audience from our developer mailing > list. > > > These > > > > > > > people > > > > > > > > > > code > > > > > > > > > > > > for a > > > > > > > > > > > > living not necessarily as a passion, and may not > > > > > understand > > > > > > > details > > > > > > > > > > of > > > > > > > > > > > > the > > > > > > > > > > > > internals of our system or even basic things like > > > > > > > multi-threaded > > > > > > > > > > > > programming. I don't think this means we want to > dumb > > > > > things > > > > > > > down, > > > > > > > > > > but > > > > > > > > > > > > rather try really hard to make things truly simple > > > when > > > > > > > possible. > > > > > > > > > > > > > > > > > > > > > > > > Okay here were a couple of comments: > > > > > > > > > > > > > > > > > > > > > > > > 1. Conceptually what is a TopicContext? I think it > > > means > > > > > > > something > > > > > > > > > > > > like > > > > > > > > > > > > TopicAdmin? It is not literally context about > Topics > > > > > right? > > > > > > > What is > > > > > > > > > > > > the > > > > > > > > > > > > relationship of Contexts to clients? Is there a > > > > > threadsafety > > > > > > > > > > > > difference? > > > > > > > > > > > > Would be nice to not have to think about this, > this is > > > > > what I > > > > > > > mean > > > > > > > > > > by > > > > > > > > > > > > "conceptual weight". We introduce a new concept > that > > > is a > > > > > bit > > > > > > > > > > nebulous > > > > > > > > > > > > that > > > > > > > > > > > > I have to figure out to use what could be a simple > > > api. > > > > > I'm > > > > > > > sure > > > > > > > > > > > > you've > > > > > > > > > > > > been through this experience before where you have > > > these > > > > > > > various > > > > > > > > > > > > objects > > > > > > > > > > > > and you're trying to figure out what they > represent > > > (the > > > > > > > connection > > > > > > > > > > to > > > > > > > > > > > > the > > > > > > > > > > > > server? the information to create a connection? a > > > request > > > > > > > session?). > > > > > > > > > > > > > > > > > > > > > > The intention was to provide some grouping of methods, > and > > > > > also a > > > > > > > place > > > > > > > > > > > to put request parameters which were often set to > defaults > > > > > rather > > > > > > > than > > > > > > > > > > > being explicitly set. If it seems complex, we can > > > certainly > > > > > get > > > > > > > rid of > > > > > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > > 2. We've tried to avoid the Impl naming > convention. In > > > > > > > general the > > > > > > > > > > > > rule > > > > > > > > > > > > has been if there is only going to be one > > > implementation > > > > > you > > > > > > > don't > > > > > > > > > > > > need an > > > > > > > > > > > > interface. If there will be multiple, distinguish > it > > > from > > > > > the > > > > > > > > > > others. > > > > > > > > > > > > The > > > > > > > > > > > > other clients follow this pattern: Producer, > > > > > KafkaProducer, > > > > > > > > > > > > MockProducer; > > > > > > > > > > > > Consumer, KafkaConsumer, MockConsumer. > > > > > > > > > > > > > > > > > > > > > > Good point. Let's change the interface to > > > KafkaAdminClient, > > > > > and > > > > > > > the > > > > > > > > > > > implementation to AdminClient. > > > > > > > > > > > > > > > > > > > > > > > 3. We generally don't use setters or getters as a > > > naming > > > > > > > > > > convention. I > > > > > > > > > > > > personally think mutating the setting in place > seems > > > kind > > > > > of > > > > > > > like > > > > > > > > > > late > > > > > > > > > > > > 90s > > > > > > > > > > > > Java style. I think it likely has thread-safety > > > issues. > > > > > i.e. > > > > > > > even if > > > > > > > > > > > > it is > > > > > > > > > > > > volatile you may not get the value you just set if > > > there > > > > > is > > > > > > > another > > > > > > > > > > > > thread... I actually really liked what you > described > > > as > > > > > your > > > > > > > > > > original > > > > > > > > > > > > idea > > > > > > > > > > > > of having a single parameter object like > > > > > CreateTopicRequest > > > > > > > that > > > > > > > > > > holds > > > > > > > > > > > > all > > > > > > > > > > > > these parameters and defaults. This lets you > evolve > > > the > > > > > api > > > > > > > with all > > > > > > > > > > > > the > > > > > > > > > > > > various combinations of arguments without > overloading > > > > > > > insanity. > > > > > > > > > > After > > > > > > > > > > > > doing > > > > > > > > > > > > literally tens of thousands of remote APIs at > > > LinkedIn we > > > > > > > eventually > > > > > > > > > > > > converged on a rule, which is ultimately every > remote > > > api > > > > > > > needs a > > > > > > > > > > > > single > > > > > > > > > > > > argument object you can add to over time and it > must > > > be > > > > > > > batched. > > > > > > > > > > Which > > > > > > > > > > > > brings me to my next point... > > > > > > > > > > > > > > > > > > > > > > Just to clarify, volatiles were never a part of the > > > proposal. > > > > > I > > > > > > > think > > > > > > > > > > > that context objects or request objects should be used > by a > > > > > single > > > > > > > > > > > thread at a time. > > > > > > > > > > > > > > > > > > > > > > I'm not opposed to request objects, but I think they > raise > > > all > > > > > the > > > > > > > same > > > > > > > > > > > questions as context objects. Basically, the > thread-safety > > > > > issues > > > > > > > need > > > > > > > > > > > to be spelled out and understood by the user, and the > user > > > > > needs > > > > > > > more > > > > > > > > > > > lines of code to make a request. And there will be > people > > > > > trying > > > > > > > to do > > > > > > > > > > > things like re-use request objects when they should > not, > > > and so > > > > > > > forth. > > > > > > > > > > > > > > > > > > > > > > > 4. I agree batch apis are annoying but I suspect > we'll > > > > > end up > > > > > > > adding > > > > > > > > > > > > one. Doing 1000 requests for 1000 operations if > > > creating > > > > > or > > > > > > > deleting > > > > > > > > > > > > will > > > > > > > > > > > > be bad, right? This won't be the common case, but > when > > > > > you do > > > > > > > it it > > > > > > > > > > > > will be > > > > > > > > > > > > a deal-breaker problem. I don't think we should > try > > > to fix > > > > > > > this one > > > > > > > > > > > > behind > > > > > > > > > > > > the scenes. > > > > > > > > > > > > 5. Are we going to do CompletableFuture (which > > > requires > > > > > java > > > > > > > 8) or > > > > > > > > > > > > normal Future? Normal Future is utterly useless > for > > > most > > > > > > > things > > > > > > > > > > other > > > > > > > > > > > > than > > > > > > > > > > > > just calling wait. If we can evolve in place from > > > Future > > > > > to > > > > > > > > > > > > CompletableFuture that is fantastic (we could do > it > > > for > > > > > the > > > > > > > producer > > > > > > > > > > > > too!). > > > > > > > > > > > > My belief was that this was binary incompatible > but I > > > > > > > actually don't > > > > > > > > > > > > know > > > > > > > > > > > > (obviously it's source compatible). > > > > > > > > > > > > > > > > > > > > > > In my testing, replacing a return type with a subclass > of > > > that > > > > > > > return > > > > > > > > > > > type did not break binary compatibility. I haven't > been > > > able > > > > > to > > > > > > > find > > > > > > > > > > > chapter and verse on this from the Java implementers, > > > though. > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -Jay > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Feb 8, 2017 at 5:00 PM, Colin McCabe < > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > I made some major revisions to the proposal on the > > > wiki, so > > > > > > > please > > > > > > > > > > check > > > > > > > > > > > > > it out. > > > > > > > > > > > > > > > > > > > > > > > > > > The new API is based on Ismael's suggestion of > grouping > > > > > > > related APIs. > > > > > > > > > > > > > There is only one layer of grouping. I think that > it's > > > > > > > actually > > > > > > > > > > pretty > > > > > > > > > > > > > intuitive. It's also based on the idea of using > > > Futures, > > > > > which > > > > > > > > > > several > > > > > > > > > > > > > people commented that they'd like to see. > > > > > > > > > > > > > > > > > > > > > > > > > > Here's a simple example: > > > > > > > > > > > > > > > > > > > > > > > > > > > AdminClient client = new > AdminClientImpl(myConfig); > > > > > > > > > > > > > > try { > > > > > > > > > > > > > > client.topics().create("foo", 3, (short) 2, > > > > > false).get(); > > > > > > > > > > > > > > Collection<String> topicNames = > > > > > > > client.topics().list(false). > > > > > > > > > > get(); > > > > > > > > > > > > > > log.info("Found topics: {}", > > > > > Utils.mkString(topicNames, > > > > > > > ", ")); > > > > > > > > > > > > > > Collection<Node> nodes = > > > client.nodes().list().get(); > > > > > > > > > > > > > > log.info("Found cluster nodes: {}", > > > > > > > Utils.mkString(nodes, ", > > > > > > > > > > ")); > > > > > > > > > > > > > > } finally { > > > > > > > > > > > > > > client.close(); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > The good thing is, there is no Try, no 'get' > prefixes, > > > no > > > > > > > messing > > > > > > > > > > with > > > > > > > > > > > > > batch APIs. If there is an error, then > Future#get() > > > > > throws an > > > > > > > > > > > > > ExecutionException which wraps the relevant > exception > > > in > > > > > the > > > > > > > standard > > > > > > > > > > > > > Java way. > > > > > > > > > > > > > > > > > > > > > > > > > > Here's a slightly less simple example: > > > > > > > > > > > > > > > > > > > > > > > > > > > AdminClient client = new > AdminClientImpl(myConfig); > > > > > > > > > > > > > > try { > > > > > > > > > > > > > > List<Future<Void>> futures = new > LinkedList<>(); > > > > > > > > > > > > > > for (String topicName: myNewTopicNames) { > > > > > > > > > > > > > > creations.add(client.topics(). > > > > > > > > > > > > > > setClientTimeout(30000).setCr > eationConfig( > > > > > > > myTopicConfig). > > > > > > > > > > > > > > create(topicName, 3, (short) 2, > false)); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > Futures.waitForAll(futures); > > > > > > > > > > > > > > } finally { > > > > > > > > > > > > > > client.close(); > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > I went with Futures because I feel like ought to > have > > > some > > > > > > > option for > > > > > > > > > > > > > doing async. It's a style of programming that has > > > become a > > > > > > > lot more > > > > > > > > > > > > > popular with the rise of Node.js, Twisted python, > etc. > > > etc. > > > > > > > Also, as > > > > > > > > > > > > > Ismael commented, Java 8 CompletableFuture is > going to > > > make > > > > > > > Java's > > > > > > > > > > > > > support for fluent async programming a lot > stronger by > > > > > > > allowing call > > > > > > > > > > > > > chaining and much more. > > > > > > > > > > > > > > > > > > > > > > > > > > If we are going to support async, the simplest > thing is > > > > > just > > > > > > > to make > > > > > > > > > > > > > everything return a future and let people call > get() if > > > > > they > > > > > > > want to > > > > > > > > > > run > > > > > > > > > > > > > synchronously. Having a mix of async and sync > APIs is > > > just > > > > > > > going to > > > > > > > > > > be > > > > > > > > > > > > > confusing and redundant. > > > > > > > > > > > > > > > > > > > > > > > > > > I think we should try to avoid creating single > > > functions > > > > > that > > > > > > > start > > > > > > > > > > > > > multiple requests if we can. It makes things much > > > > > uglier. It > > > > > > > means > > > > > > > > > > > > > that you have to have some kind of request class > that > > > > > wraps up > > > > > > > the > > > > > > > > > > > > > request the user is trying to create, so that you > can > > > > > handle an > > > > > > > > > > array of > > > > > > > > > > > > > those requests. The return value has to be > something > > > like > > > > > > > Map<Node, > > > > > > > > > > > > > Try<Value>> to represent which nodes failed and > > > succeeded. > > > > > > > This is > > > > > > > > > > the > > > > > > > > > > > > > kind of stuff that, in my opinion, makes people > scratch > > > > > their > > > > > > > heads. > > > > > > > > > > > > > > > > > > > > > > > > > > If we need to, we can still get some of the > efficiency > > > > > > > benefits of > > > > > > > > > > batch > > > > > > > > > > > > > APIs by waiting for a millisecond or two before > sending > > > > > out a > > > > > > > topic > > > > > > > > > > > > > create() request to see if other create() requests > > > > > arrive. If > > > > > > > so, we > > > > > > > > > > > > > can coalesce them. It might be better to figure > out if > > > > > this > > > > > > > is an > > > > > > > > > > > > > actual performance issue before implementing it, > > > though. > > > > > > > > > > > > > > > > > > > > > > > > > > I think it would be good to get something out > there, > > > > > annotate > > > > > > > it as > > > > > > > > > > > > > @Unstable, and get feedback from people building > > > against > > > > > trunk > > > > > > > and > > > > > > > > > > using > > > > > > > > > > > > > it. We have removed or changed @Unstable APIs in > > > streams > > > > > > > before, so > > > > > > > > > > I > > > > > > > > > > > > > don't think we should worry that it will get set in > > > stone > > > > > > > > > > prematurely. > > > > > > > > > > > > > The AdminClient API should get much less developer > use > > > than > > > > > > > anything > > > > > > > > > > in > > > > > > > > > > > > > streams, so changing an unstable API should be much > > > easier. > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Feb 8, 2017, at 07:49, Ismael Juma wrote: > > > > > > > > > > > > > > Thanks for elaborating Jay. I totally agree that > we > > > have > > > > > to > > > > > > > be very > > > > > > > > > > > > > > careful > > > > > > > > > > > > > > in how we use our complexity budget. Easier said > than > > > > > done > > > > > > > when > > > > > > > > > > people > > > > > > > > > > > > > > don't agree on what is complex and what is > simple. > > > :) For > > > > > > > example, > > > > > > > > > > I > > > > > > > > > > > > > > think > > > > > > > > > > > > > > batch APIs are a significant source of > complexity as > > > you > > > > > > > have to > > > > > > > > > > do a > > > > > > > > > > > > > > bunch > > > > > > > > > > > > > > of ceremony to group things before sending the > > > request > > > > > and > > > > > > > error > > > > > > > > > > handling > > > > > > > > > > > > > > becomes more complex due to partial failures > (things > > > like > > > > > > > `Try` or > > > > > > > > > > other > > > > > > > > > > > > > > mechanisms that serve a similar role are then > > > needed). > > > > > > > > > > > > > > > > > > > > > > > > > > > > Maybe a way forward is to write API usage > examples to > > > > > help > > > > > > > > > > validate that > > > > > > > > > > > > > > the suggested API is indeed easy to use. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Feb 8, 2017 at 4:40 AM, Jay Kreps < > > > > > j...@confluent.io> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Totally agree on CompletableFuture. Also agree > with > > > > > some > > > > > > > of the > > > > > > > > > > rough > > > > > > > > > > > > > edges > > > > > > > > > > > > > > > on the Consumer. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't have much of a leg to stand on with the > > > > > splitting > > > > > > > vs not > > > > > > > > > > > > > splitting > > > > > > > > > > > > > > > thing, really hard to argue one or the other is > > > > > better. I > > > > > > > guess > > > > > > > > > > the one > > > > > > > > > > > > > > > observation in watching us try to make good > public > > > apis > > > > > > > over the > > > > > > > > > > years > > > > > > > > > > > > > is I > > > > > > > > > > > > > > > am kind of in favor of a particular kind of > > > simple. In > > > > > > > > > > particular I > > > > > > > > > > > > > think > > > > > > > > > > > > > > > since the bar is sooo high in support and docs > and > > > the > > > > > > > community > > > > > > > > > > of > > > > > > > > > > > > > users > > > > > > > > > > > > > > > so broad in the range of their capabilities, it > > > makes > > > > > it so > > > > > > > > > > there is a > > > > > > > > > > > > > lot > > > > > > > > > > > > > > > of value in dead simple interfaces that don't > have > > > a > > > > > lot of > > > > > > > > > > conceptual > > > > > > > > > > > > > > > weight, don't introduce a lot of new classes or > > > > > concepts or > > > > > > > > > > general > > > > > > > > > > > > > > > patterns that must be understood to use them > > > > > correctly. So > > > > > > > > > > things like > > > > > > > > > > > > > > > nesting, or the Try class, or async apis, or > even > > > just > > > > > a > > > > > > > complex > > > > > > > > > > set of > > > > > > > > > > > > > > > classes representing arguments or return values > > > kind of > > > > > > > all stack > > > > > > > > > > > > > > > conceptual burdens on the user to figure out > > > correct > > > > > > > usage. So > > > > > > > > > > like, > > > > > > > > > > > > > for > > > > > > > > > > > > > > > example, the Try class is very elegant and > > > represents a > > > > > > > whole > > > > > > > > > > > > > generalized > > > > > > > > > > > > > > > class of possibly completed actions, but the > flip > > > side > > > > > is > > > > > > > maybe > > > > > > > > > > I'm > > > > > > > > > > > > > just a > > > > > > > > > > > > > > > working guy who needs to list his kafka topics > but > > > > > doesn't > > > > > > > know > > > > > > > > > > Rust, > > > > > > > > > > > > > take > > > > > > > > > > > > > > > pity on me! :-) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Nit picking aside, super excited to see us > > > progress on > > > > > > > this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -Jay > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Feb 7, 2017 at 3:46 PM Ismael Juma < > > > > > > > ism...@juma.me.uk> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jay, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the feedback. Comments inline. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Feb 7, 2017 at 8:18 PM, Jay Kreps < > > > > > > > j...@confluent.io> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - I think it would be good to not use > "get" > > > as > > > > > the > > > > > > > prefix > > > > > > > > > > for > > > > > > > > > > > > > things > > > > > > > > > > > > > > > > > making remote calls. We've tried to > avoid > > > the > > > > > java > > > > > > > getter > > > > > > > > > > > > > convention > > > > > > > > > > > > > > > > > entirely (see code style guide), but for > > > remote > > > > > > > calls in > > > > > > > > > > > > > particular > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > kind > > > > > > > > > > > > > > > > > of blurs the line between field access > and > > > > > remote > > > > > > > RPC in > > > > > > > > > > a way > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > leads > > > > > > > > > > > > > > > > > people to trouble. What about, e.g., > > > > > > > fetchAllGroups() vs > > > > > > > > > > > > > > > > getAllGroups(). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Agreed that we should avoid the `get` prefix > for > > > > > remote > > > > > > > calls. > > > > > > > > > > There > > > > > > > > > > > > > are > > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > few possible prefixes for the read > operations: > > > list, > > > > > > > fetch, > > > > > > > > > > describe. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - I think futures and callbacks are a > bit > > > of a > > > > > pain > > > > > > > to > > > > > > > > > > use. I'd > > > > > > > > > > > > > > > second > > > > > > > > > > > > > > > > > Becket's comment: let's ensure there a > > > common > > > > > use > > > > > > > case > > > > > > > > > > > > > motivating > > > > > > > > > > > > > > > > these > > > > > > > > > > > > > > > > > that wouldn't be just as easily > satisfied > > > with > > > > > batch > > > > > > > > > > operations > > > > > > > > > > > > > > > (which > > > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > seem to have at least for some things). > In > > > > > terms of > > > > > > > > > > flexibility > > > > > > > > > > > > > > > > > Callbacks > > > > > > > > > > > > > > > > > > Futures > Batch Ops but I think in > terms of > > > > > > > usability it > > > > > > > > > > is the > > > > > > > > > > > > > > > exact > > > > > > > > > > > > > > > > > opposite so let's make sure we have > worked > > > out > > > > > how > > > > > > > the > > > > > > > > > > API will > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > used > > > > > > > > > > > > > > > > > before deciding. In particular I think > java > > > > > Futures > > > > > > > are > > > > > > > > > > often an > > > > > > > > > > > > > > > > > uncomfortable half-way point since > calling > > > > > get() and > > > > > > > > > > blocking > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > thread is > > > > > > > > > > > > > > > > > often not what you want for chaining > > > sequences > > > > > of > > > > > > > > > > operations in > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > truly > > > > > > > > > > > > > > > > > async way, so 99% of people just use the > > > future > > > > > as > > > > > > > a way > > > > > > > > > > to > > > > > > > > > > > > > batch > > > > > > > > > > > > > > > > calls. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We should definitely figure out how the APIs > are > > > > > going > > > > > > > to be > > > > > > > > > > used > > > > > > > > > > > > > before > > > > > > > > > > > > > > > > deciding. I agree that callbacks are > definitely > > > > > painful > > > > > > > and > > > > > > > > > > there's > > > > > > > > > > > > > > > little > > > > > > > > > > > > > > > > reason to expose them in a modern API unless > it's > > > > > meant > > > > > > > to be > > > > > > > > > > very > > > > > > > > > > > > > low > > > > > > > > > > > > > > > > level. When it comes to Futures, I think it's > > > > > important > > > > > > > to > > > > > > > > > > > > > distinguish > > > > > > > > > > > > > > > what > > > > > > > > > > > > > > > > is available in Java 7 and below versus what > is > > > > > > > available from > > > > > > > > > > Java 8 > > > > > > > > > > > > > > > > onwards. CompletableFuture makes it much > easier > > > to > > > > > > > > > > compose/chain > > > > > > > > > > > > > > > operations > > > > > > > > > > > > > > > > (in a similar vein to java.util.Stream, our > own > > > > > Streams > > > > > > > API, > > > > > > > > > > etc.) > > > > > > > > > > > > > and it > > > > > > > > > > > > > > > > gives you the ability to register callbacks > if > > > you > > > > > > > really want > > > > > > > > > > to > > > > > > > > > > > > > > > (avoiding > > > > > > > > > > > > > > > > the somewhat odd situation in the Producer > where > > > we > > > > > > > return a > > > > > > > > > > Future > > > > > > > > > > > > > _and_ > > > > > > > > > > > > > > > > allow you to pass a callback). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - Personally I don't think splitting the > > > admin > > > > > > > methods up > > > > > > > > > > > > > actually > > > > > > > > > > > > > > > > makes > > > > > > > > > > > > > > > > > things more usable. It just makes you > have > > > to > > > > > dig > > > > > > > through > > > > > > > > > > our > > > > > > > > > > > > > > > > > hierarchy. I > > > > > > > > > > > > > > > > > think a flat class with a bunch of > > > operations > > > > > (like > > > > > > > the > > > > > > > > > > consumer > > > > > > > > > > > > > > > api) > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > probably the easiest for people to grok > and > > > find > > > > > > > things > > > > > > > > > > on. I am > > > > > > > > > > > > > > > kind > > > > > > > > > > > > > > > > > of a > > > > > > > > > > > > > > > > > dumb PHP programmer at heart, though. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am not sure it's fair to compare the > > > AdminClient > > > > > with > > > > > > > the > > > > > > > > > > > > > Consumer. The > > > > > > > > > > > > > > > > former has APIs for a bunch of unrelated APIs > > > > > (topics, > > > > > > > ACLs, > > > > > > > > > > configs, > > > > > > > > > > > > > > > > consumer groups, delegation tokens, preferred > > > leader > > > > > > > election, > > > > > > > > > > > > > partition > > > > > > > > > > > > > > > > reassignment, etc.) where the latter is > pretty > > > > > > > specialised. > > > > > > > > > > For each > > > > > > > > > > > > > of > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > resources, you may have 3-4 operations, it > will > > > get > > > > > > > confusing > > > > > > > > > > fast. > > > > > > > > > > > > > Also, > > > > > > > > > > > > > > > > do you really think an API that has one > level of > > > > > > > grouping will > > > > > > > > > > mean > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > users have to "dig through our hierarchy"? Or > > > are you > > > > > > > > > > concerned that > > > > > > > > > > > > > once > > > > > > > > > > > > > > > > we go in that direction, there is a danger of > > > making > > > > > the > > > > > > > > > > hierarchy > > > > > > > > > > > > > more > > > > > > > > > > > > > > > > complicated? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Finally, I am not sure I would use the > consumer > > > as an > > > > > > > example > > > > > > > > > > of > > > > > > > > > > > > > > > something > > > > > > > > > > > > > > > > that is easy to grok. :) The fact that > methods > > > behave > > > > > > > pretty > > > > > > > > > > > > > differently > > > > > > > > > > > > > > > > (some are blocking while others only have an > > > effect > > > > > after > > > > > > > > > > poll) with > > > > > > > > > > > > > no > > > > > > > > > > > > > > > > indication from the type signature or naming > > > > > convention > > > > > > > makes > > > > > > > > > > it > > > > > > > > > > > > > harder, > > > > > > > > > > > > > > > > not easier, to understand. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
