Cool. Yeah +1 on make it consistent. Thanks,
Mayuresh On Fri, Jul 24, 2015 at 7:29 PM, Grant Henke <ghe...@cloudera.com> wrote: > They take precedence in order of granularity (ms > minutes > hours). Below > is the relevant snippet of code. > > I vote for keeping all time related configs in milliseconds and deprecating > the others. It gives you the most control in a single config. The minutes > and hours ones offer no new functionality that can't be achieved with > the milliseconds one. > > private def getLogRetentionTimeMillis(): Long = { > val millisInMinute = 60L * 1000L > val millisInHour = 60L * millisInMinute > > if(props.containsKey("log.retention.ms")){ > props.getIntInRange("log.retention.ms", (1, Int.MaxValue)) > } > else if(props.containsKey("log.retention.minutes")){ > millisInMinute * props.getIntInRange("log.retention.minutes", (1, > Int.MaxValue)) > } > else { > millisInHour * props.getIntInRange("log.retention.hours", 24*7, (1, > Int.MaxValue)) > } > } > > On Fri, Jul 24, 2015 at 9:18 PM, Mayuresh Gharat < > gharatmayures...@gmail.com > > wrote: > > > Hmm. > > This creates confusion for people who might be new to kafka or have never > > used those properties. > > Does deprecating the older ones make sense here? > > > > Thanks, > > > > Mayuresh > > > > > > On Fri, Jul 24, 2015 at 7:15 PM, Gwen Shapira <gshap...@cloudera.com> > > wrote: > > > > > Agree. All I found on this was KAFKA-1325, which is more about > > > inconsistency than real use-case. > > > > > > Anyway, I'd argue that if we added it in 0.8.2.0, we can't take it out > > now > > > :) > > > > > > On Fri, Jul 24, 2015 at 7:04 PM, Aditya Auradkar > > > <aaurad...@linkedin.com.invalid> wrote: > > > > Is there actually a use case where we need "log.retention.ms"? In > most > > > > cases, people would want to retain their logs for at least a few > > minutes > > > > I'd think. > > > > > > > > Aditya > > > > > > > > On Fri, Jul 24, 2015 at 6:49 PM, Gwen Shapira <gshap...@cloudera.com > > > > > wrote: > > > > > > > >> Backward compatibility, I think. > > > >> > > > >> At least the "ms" one is fairly new, and I think we left the others > to > > > >> avoid break configuration during upgrade. > > > >> > > > >> On Fri, Jul 24, 2015 at 6:34 PM, Mayuresh Gharat > > > >> <gharatmayures...@gmail.com> wrote: > > > >> > I was thinking why we have 3 different configs for the same > property > > > (log > > > >> > retention) : > > > >> > > > > >> > "log.retention.ms" > > > >> > "log.retention.minutes" > > > >> > "log.retention.hours" > > > >> > > > > >> > Why don't we only use the Milliseconds? > > > >> > There are other properties as well like log Jitter, LogRollTime > > which > > > >> raise > > > >> > the same question in my mind. > > > >> > > > > >> > The reason I am asking this is that a user might misconfigure them > > > like > > > >> > setting the log.retention.ms = 1 and log.retention.hours = 1. > > > >> > > > > >> > -- > > > >> > -Regards, > > > >> > Mayuresh R. Gharat > > > >> > (862) 250-7125 > > > >> > > > > > > > > > > > -- > > -Regards, > > Mayuresh R. Gharat > > (862) 250-7125 > > > > > > -- > Grant Henke > Solutions Consultant | Cloudera > ghe...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke > -- -Regards, Mayuresh R. Gharat (862) 250-7125