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

Reply via email to