I see we have the similar setting for CreateTopic request timeout <= 0 as well, so maybe it has been discussed and I simply overlooked.. otherwise my question is for both of these cases.
Guozhang On Tue, Jun 21, 2016 at 4:07 PM, Guozhang Wang <wangg...@gmail.com> wrote: > Thanks Grant, looks good to me overall. One minor comment below: > > > - The error code in the response will either contain an argument > > validation exception or a timeout exception. If you receive a > timeout > > exception, because you asked for 0 timeout, you can assume the > message was > > valid and the topic deletion was triggered. > > In the timeout <= 0 case, if the client should always ignore and treat the > timeout > error code as "OK", should we just return no error code in this case? > > > Guozhang > > > On Tue, Jun 21, 2016 at 8:17 AM, Grant Henke <ghe...@cloudera.com> wrote: > >> Hi Ismael, >> >> Thanks for the review. See my responses below. >> >> One potential issue is that the number of topics in the response won't >> > match the number of topics in the request. Worth checking what others >> think >> > of this one. >> >> >> Based on the choice of how we handled duplicate topics in the create >> topics >> protocol, I took the same approach here. At one point create topics would >> disconnect because I could't return an error per topic request. In the end >> the preference was to return and error code even though the cardinality >> would be different. >> >> 4. When requesting to delete a topic that is already marked for >> > > > deletion, the request will wait for the wait for the timeout and >> > > return as >> > > > usual >> > >> > Do you mean that it will wait up to the timeout until the delete is >> > "complete" as per the definition in `6`? Or will it wait unconditionally >> > until the timeout expires? It would be good to make that clear. >> > >> >> That's exactly what I meant. I updated the wiki. >> >> This could leak topic name information (as per KAFKA-3396, which was filed >> > by you). We would probably want to return `InvalidTopic` for the case >> where >> > the user doesn't have a valid `DESCRIBE TOPIC` ACL, right? >> >> >> Good point. I will update the wiki and patch. >> >> Unauthorized requests will receive a TopicAuthorizationException if they >> are authorized to the "Describe" Operation on the "Topic" resource. >> Otherwise they will receive an InvalidTopicException as if the topic does >> not exist. >> >> I was wondering (and this applies to the create topic as well), is there >> > any value in a flag that says whether the timeout expired or not? >> >> >> In both the create and delete response we return a TimeoutException error >> code for the topics that did not "complete" in time. This allows the >> client >> to know which topics actions completed and which timed out. I updated the >> wiki to explicitly call this out in the response section. >> >> Thanks, >> Grant >> >> On Tue, Jun 21, 2016 at 5:44 AM, Ismael Juma <ism...@juma.me.uk> wrote: >> >> > Thanks Grant. A few comments inline. >> > >> > On Mon, Jun 20, 2016 at 9:09 PM, Grant Henke <ghe...@cloudera.com> >> wrote: >> > >> > > > 2. If there are multiple instructions for the same topic in one >> > > > request the extra request will be ignored >> > > > - This is because the list of topics is modeled server side >> as a >> > > set >> > > > - Multiple deletes results in the same end goal, so handling >> this >> > > > error for the user should be okay >> > > >> > >> > One potential issue is that the number of topics in the response won't >> > match the number of topics in the request. Worth checking what others >> think >> > of this one. >> > >> > 4. When requesting to delete a topic that is already marked for >> > > > deletion, the request will wait for the wait for the timeout and >> > > return as >> > > > usual >> > >> > >> > Do you mean that it will wait up to the timeout until the delete is >> > "complete" as per the definition in `6`? Or will it wait unconditionally >> > until the timeout expires? It would be good to make that clear. >> > >> > >> > > > 5. The principal must be authorized to the "Delete" Operation on >> the >> > > >> > > "Topic" resource to delete the topic. >> > > > - Unauthorized requests will receive a >> > TopicAuthorizationException >> > > >> > >> > This could leak topic name information (as per KAFKA-3396, which was >> filed >> > by you). We would probably want to return `InvalidTopic` for the case >> where >> > the user doesn't have a valid `DESCRIBE TOPIC` ACL, right? >> > >> > >> > > > - Why have a timeout at all? Deletes could take a while? >> > > >> > >> > I was wondering (and this applies to the create topic as well), is there >> > any value in a flag that says whether the timeout expired or not? >> > >> > Thanks, >> > Ismael >> > >> >> >> >> -- >> Grant Henke >> Software Engineer | Cloudera >> gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke >> > > > > -- > -- Guozhang > -- -- Guozhang
