Hi Eno, Sorry for the delayed response. - I haven't implemented the feature yet, so no experimental results so far. And I plan to test in out in the following days.
- You are absolutely right that the priority queue does not completely prevent data requests being processed ahead of controller requests. That being said, I expect it to greatly mitigate the effect of stable metadata. In any case, I'll try it out and post the results when I have it. Regards, Lucas On Wed, Jun 20, 2018 at 5:44 AM, Eno Thereska <eno.there...@gmail.com> wrote: > Hi Lucas, > > Sorry for the delay, just had a look at this. A couple of questions: > - did you notice any positive change after implementing this KIP? I'm > wondering if you have any experimental results that show the benefit of the > two queues. > > - priority is usually not sufficient in addressing the problem the KIP > identifies. Even with priority queues, you will sometimes (often?) have the > case that data plane requests will be ahead of the control plane requests. > This happens because the system might have already started processing the > data plane requests before the control plane ones arrived. So it would be > good to know what % of the problem this KIP addresses. > > Thanks > Eno > > On Fri, Jun 15, 2018 at 4:44 PM, Ted Yu <yuzhih...@gmail.com> wrote: > > > Change looks good. > > > > Thanks > > > > On Fri, Jun 15, 2018 at 8:42 AM, Lucas Wang <lucasatu...@gmail.com> > wrote: > > > > > Hi Ted, > > > > > > Thanks for the suggestion. I've updated the KIP. Please take another > > look. > > > > > > Lucas > > > > > > On Thu, Jun 14, 2018 at 6:34 PM, Ted Yu <yuzhih...@gmail.com> wrote: > > > > > > > Currently in KafkaConfig.scala : > > > > > > > > val QueuedMaxRequests = 500 > > > > > > > > It would be good if you can include the default value for this new > > config > > > > in the KIP. > > > > > > > > Thanks > > > > > > > > On Thu, Jun 14, 2018 at 4:28 PM, Lucas Wang <lucasatu...@gmail.com> > > > wrote: > > > > > > > > > Hi Ted, Dong > > > > > > > > > > I've updated the KIP by adding a new config, instead of reusing the > > > > > existing one. > > > > > Please take another look when you have time. Thanks a lot! > > > > > > > > > > Lucas > > > > > > > > > > On Thu, Jun 14, 2018 at 2:33 PM, Ted Yu <yuzhih...@gmail.com> > wrote: > > > > > > > > > > > bq. that's a waste of resource if control request rate is low > > > > > > > > > > > > I don't know if control request rate can get to 100,000, likely > > not. > > > > Then > > > > > > using the same bound as that for data requests seems high. > > > > > > > > > > > > On Wed, Jun 13, 2018 at 10:13 PM, Lucas Wang < > > lucasatu...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > Hi Ted, > > > > > > > > > > > > > > Thanks for taking a look at this KIP. > > > > > > > Let's say today the setting of "queued.max.requests" in > cluster A > > > is > > > > > > 1000, > > > > > > > while the setting in cluster B is 100,000. > > > > > > > The 100 times difference might have indicated that machines in > > > > cluster > > > > > B > > > > > > > have larger memory. > > > > > > > > > > > > > > By reusing the "queued.max.requests", the controlRequestQueue > in > > > > > cluster > > > > > > B > > > > > > > automatically > > > > > > > gets a 100x capacity without explicitly bothering the > operators. > > > > > > > I understand the counter argument can be that maybe that's a > > waste > > > of > > > > > > > resource if control request > > > > > > > rate is low and operators may want to fine tune the capacity of > > the > > > > > > > controlRequestQueue. > > > > > > > > > > > > > > I'm ok with either approach, and can change it if you or anyone > > > else > > > > > > feels > > > > > > > strong about adding the extra config. > > > > > > > > > > > > > > Thanks, > > > > > > > Lucas > > > > > > > > > > > > > > > > > > > > > On Wed, Jun 13, 2018 at 3:11 PM, Ted Yu <yuzhih...@gmail.com> > > > wrote: > > > > > > > > > > > > > > > Lucas: > > > > > > > > Under Rejected Alternatives, #2, can you elaborate a bit more > > on > > > > why > > > > > > the > > > > > > > > separate config has bigger impact ? > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > On Wed, Jun 13, 2018 at 2:00 PM, Dong Lin < > lindon...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hey Luca, > > > > > > > > > > > > > > > > > > Thanks for the KIP. Looks good overall. Some comments > below: > > > > > > > > > > > > > > > > > > - We usually specify the full mbean for the new metrics in > > the > > > > KIP. > > > > > > Can > > > > > > > > you > > > > > > > > > specify it in the Public Interface section similar to > KIP-237 > > > > > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > > > 237%3A+More+Controller+Health+Metrics> > > > > > > > > > ? > > > > > > > > > > > > > > > > > > - Maybe we could follow the same pattern as KIP-153 > > > > > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > > > 153%3A+Include+only+client+traffic+in+BytesOutPerSec+ > > metric>, > > > > > > > > > where we keep the existing sensor name "BytesInPerSec" and > > add > > > a > > > > > new > > > > > > > > sensor > > > > > > > > > "ReplicationBytesInPerSec", rather than replacing the > sensor > > > > name " > > > > > > > > > BytesInPerSec" with e.g. "ClientBytesInPerSec". > > > > > > > > > > > > > > > > > > - It seems that the KIP changes the semantics of the broker > > > > config > > > > > > > > > "queued.max.requests" because the number of total requests > > > queued > > > > > in > > > > > > > the > > > > > > > > > broker will be no longer bounded by "queued.max.requests". > > This > > > > > > > probably > > > > > > > > > needs to be specified in the Public Interfaces section for > > > > > > discussion. > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Dong > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Jun 13, 2018 at 12:45 PM, Lucas Wang < > > > > > lucasatu...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Kafka experts, > > > > > > > > > > > > > > > > > > > > I created KIP-291 to add a separate queue for controller > > > > > requests: > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > 291% > > > > > > > > > > 3A+Have+separate+queues+for+control+requests+and+data+ > > > requests > > > > > > > > > > > > > > > > > > > > Can you please take a look and let me know your feedback? > > > > > > > > > > > > > > > > > > > > Thanks a lot for your time! > > > > > > > > > > Regards, > > > > > > > > > > Lucas > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
