Hi,

Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
(including the ones from hbase-default.xml) - when present. But in some
places we do not consider this, for example in HConnectionManager:

  static {
    // We set instances to one more than the value specified for {@link
    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default
max
    // connections to the ensemble from the one client is 30, so in that
case we
    // should run into zk issues before the LRU hit this value of 31.
    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
HConnectionImplementation>(
        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
       @Override
      protected boolean removeEldestEntry(
          Map.Entry<HConnectionKey, HConnectionImplementation> eldest) {
         return size() > MAX_CACHED_HBASE_INSTANCES;
       }
    };
This only reads it from hbase-site.xml+hbase-default.xml. This is
inconsistent, I think this should use ZKConfig.makeZKProps(conf) and then
get the value.

Thoughts?

Lars

I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
properties is different from the way we name
"hbase.zookeeper.property.maxClientCnxns"

e.g.
# the port at which the clients will connect
clientPort=2181

FYI

On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]> wrote:

> Hi,
>
> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
> (including the ones from hbase-default.xml) - when present. But in some
> places we do not consider this, for example in HConnectionManager:
>
>  static {
>    // We set instances to one more than the value specified for {@link
>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default
> max
>    // connections to the ensemble from the one client is 30, so in that
> case we
>    // should run into zk issues before the LRU hit this value of 31.
>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
>        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
>    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
> HConnectionImplementation>(
>        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
>       @Override
>      protected boolean removeEldestEntry(
>          Map.Entry<HConnectionKey, HConnectionImplementation> eldest) {
>         return size() > MAX_CACHED_HBASE_INSTANCES;
>       }
>    };
>
>
> This only reads it from hbase-site.xml+hbase-default.xml. This is
> inconsistent, I think this should use ZKConfig.makeZKProps(conf) and then
> get the value.
>
> Thoughts?
>
> Lars
>

Should just fully deprecate zoo.cfg, it ended up being more trouble
than it was worth.  When you use zoo.cfg you cannot connect to more
than 1 cluster from a single JVM.  Annoying!

On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
> I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
> properties is different from the way we name
> "hbase.zookeeper.property.maxClientCnxns"
>
> e.g.
> # the port at which the clients will connect
> clientPort=2181
>
> FYI
>
> On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]> wrote:
>
>> Hi,
>>
>> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
>> (including the ones from hbase-default.xml) - when present. But in some
>> places we do not consider this, for example in HConnectionManager:
>>
>>  static {
>>    // We set instances to one more than the value specified for {@link
>>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default
>> max
>>    // connections to the ensemble from the one client is 30, so in that
>> case we
>>    // should run into zk issues before the LRU hit this value of 31.
>>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
>>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
>>        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
>>    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
>> HConnectionImplementation>(
>>        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
>>       @Override
>>      protected boolean removeEldestEntry(
>>          Map.Entry<HConnectionKey, HConnectionImplementation> eldest) {
>>         return size() > MAX_CACHED_HBASE_INSTANCES;
>>       }
>>    };
>>
>>
>> This only reads it from hbase-site.xml+hbase-default.xml. This is
>> inconsistent, I think this should use ZKConfig.makeZKProps(conf) and then
>> get the value.
>>
>> Thoughts?
>>
>> Lars
>>
>

Isn't that kind of the point though? If you drop in a zk config file on a
machine, you should be able to update all your apps on that machine to the
new config.
Whats more important though is being able to easily distribute a changed zk
config across your cluster and simultaneously across multiple applications.
Rather than rewriting the confs for a handful of applications, and possibly
making a mistake dealing with each application's own special semantics, and
single conf to update everything just makes sense.

I would lobby then that we make usage more consistent (as Lars recommends)
and make some of the hbase conf values to more closely match the zk conf
values (though hbase.${zk.value} is really not bad).

-Jesse
From: Ryan Rawson [[EMAIL PROTECTED]]
> Sent: Monday, July 04, 2011 5:25 AM
> To: [EMAIL PROTECTED]
> Subject: Re: zoo.cfg vs hbase-site.xml
>
> Should just fully deprecate zoo.cfg, it ended up being more trouble
> than it was worth.  When you use zoo.cfg you cannot connect to more
> than 1 cluster from a single JVM.  Annoying!
>
> On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
> > I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
> > properties is different from the way we name
> > "hbase.zookeeper.property.maxClientCnxns"
> >
> > e.g.
> > # the port at which the clients will connect
> > clientPort=2181
> >
> > FYI
> >
> > On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]>
> wrote:
> >
> >> Hi,
> >>
> >> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
> >> (including the ones from hbase-default.xml) - when present. But in some
> >> places we do not consider this, for example in HConnectionManager:
> >>
> >>  static {
> >>    // We set instances to one more than the value specified for {@link
> >>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default
> >> max
> >>    // connections to the ensemble from the one client is 30, so in that
> >> case we
> >>    // should run into zk issues before the LRU hit this value of 31.
> >>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
> >>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
> >>        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
> >>    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
> >> HConnectionImplementation>(
> >>        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
> >>       @Override
> >>      protected boolean removeEldestEntry(
> >>          Map.Entry<HConnectionKey, HConnectionImplementation> eldest) {
> >>         return size() > MAX_CACHED_HBASE_INSTANCES;
> >>       }
> >>    };
> >>
> >>
> >> This only reads it from hbase-site.xml+hbase-default.xml. This is
> >> inconsistent, I think this should use ZKConfig.makeZKProps(conf) and
> then
> >> get the value.
> >>
> >> Thoughts?
> >>
> >> Lars
> >>
> >
>

I agree that we should be more consistent in how we get zk config
(Your original report looks like a bug Lars).  I also recently tripped
over the fact that hbase uses different names for one or two zk
configs.  We need to fix that too.
St.Ack

On Mon, Jul 4, 2011 at 8:59 AM, Jesse Yates <[EMAIL PROTECTED]> wrote:
> Isn't that kind of the point though? If you drop in a zk config file on a
> machine, you should be able to update all your apps on that machine to the
> new config.
> Whats more important though is being able to easily distribute a changed zk
> config across your cluster and simultaneously across multiple applications.
> Rather than rewriting the confs for a handful of applications, and possibly
> making a mistake dealing with each application's own special semantics, and
> single conf to update everything just makes sense.
>
> I would lobby then that we make usage more consistent (as Lars recommends)
> and make some of the hbase conf values to more closely match the zk conf
> values (though hbase.${zk.value} is really not bad).
>
> -Jesse
>
>
> From: Ryan Rawson [[EMAIL PROTECTED]]
>> Sent: Monday, July 04, 2011 5:25 AM
>> To: [EMAIL PROTECTED]
>> Subject: Re: zoo.cfg vs hbase-site.xml
>>
>> Should just fully deprecate zoo.cfg, it ended up being more trouble
>> than it was worth.  When you use zoo.cfg you cannot connect to more
>> than 1 cluster from a single JVM.  Annoying!
>>
>> On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
>> > I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
>> > properties is different from the way we name
>> > "hbase.zookeeper.property.maxClientCnxns"
>> >
>> > e.g.
>> > # the port at which the clients will connect
>> > clientPort=2181
>> >
>> > FYI
>> >
>> > On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]>
>> wrote:
>> >
>> >> Hi,
>> >>
>> >> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
>> >> (including the ones from hbase-default.xml) - when present. But in some
>> >> places we do not consider this, for example in HConnectionManager:
>> >>
>> >>  static {
>> >>    // We set instances to one more than the value specified for {@link
>> >>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default
>> >> max
>> >>    // connections to the ensemble from the one client is 30, so in that
>> >> case we
>> >>    // should run into zk issues before the LRU hit this value of 31.
>> >>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
>> >>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
>> >>        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
>> >>    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
>> >> HConnectionImplementation>(
>> >>        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
>> >>       @Override
>> >>      protected boolean removeEldestEntry(
>> >>          Map.Entry<HConnectionKey, HConnectionImplementation> eldest) {
>> >>         return size() > MAX_CACHED_HBASE_INSTANCES;
>> >>       }
>> >>    };
>> >>
>> >>
>> >> This only reads it from hbase-site.xml+hbase-default.xml. This is
>> >> inconsistent, I think this should use ZKConfig.makeZKProps(conf) and
>> then
>> >> get the value.
>> >>
>> >> Thoughts?
>> >>
>> >> Lars
>> >>
>> >
>>
>

I was thinking that perhaps the normative use case for talking to a cluster
is to specify the quorum name and path... The implicit config can be really
confusing and is out of norms compared to other data store systems. Eg MySQL
memcache etc.
On Jul 6, 2011 2:14 PM, "Stack" <[EMAIL PROTECTED]> wrote:
> I agree that we should be more consistent in how we get zk config
> (Your original report looks like a bug Lars). I also recently tripped
> over the fact that hbase uses different names for one or two zk
> configs. We need to fix that too.
> St.Ack
>
> On Mon, Jul 4, 2011 at 8:59 AM, Jesse Yates <[EMAIL PROTECTED]>
wrote:
>> Isn't that kind of the point though? If you drop in a zk config file on a
>> machine, you should be able to update all your apps on that machine to
the
>> new config.
>> Whats more important though is being able to easily distribute a changed
zk
>> config across your cluster and simultaneously across multiple
applications.
>> Rather than rewriting the confs for a handful of applications, and
possibly
>> making a mistake dealing with each application's own special semantics,
and
>> single conf to update everything just makes sense.
>>
>> I would lobby then that we make usage more consistent (as Lars
recommends)
>> and make some of the hbase conf values to more closely match the zk conf
>> values (though hbase.${zk.value} is really not bad).
>>
>> -Jesse
>>
>>
>> From: Ryan Rawson [[EMAIL PROTECTED]]
>>> Sent: Monday, July 04, 2011 5:25 AM
>>> To: [EMAIL PROTECTED]
>>> Subject: Re: zoo.cfg vs hbase-site.xml
>>>
>>> Should just fully deprecate zoo.cfg, it ended up being more trouble
>>> than it was worth.  When you use zoo.cfg you cannot connect to more
>>> than 1 cluster from a single JVM.  Annoying!
>>>
>>> On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
>>> > I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
>>> > properties is different from the way we name
>>> > "hbase.zookeeper.property.maxClientCnxns"
>>> >
>>> > e.g.
>>> > # the port at which the clients will connect
>>> > clientPort=2181
>>> >
>>> > FYI
>>> >
>>> > On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]>
>>> wrote:
>>> >
>>> >> Hi,
>>> >>
>>> >> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
>>> >> (including the ones from hbase-default.xml) - when present. But in
some
>>> >> places we do not consider this, for example in HConnectionManager:
>>> >>
>>> >>  static {
>>> >>    // We set instances to one more than the value specified for
{@link
>>> >>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk
default
>>> >> max
>>> >>    // connections to the ensemble from the one client is 30, so in
that
>>> >> case we
>>> >>    // should run into zk issues before the LRU hit this value of 31.
>>> >>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
>>> >>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,
>>> >>        HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1;
>>> >>    HBASE_INSTANCES = new LinkedHashMap<HConnectionKey,
>>> >> HConnectionImplementation>(
>>> >>        (int) (MAX_CACHED_HBASE_INSTANCES / 0.75F) + 1, 0.75F, true) {
>>> >>       @Override
>>> >>      protected boolean removeEldestEntry(
>>> >>          Map.Entry<HConnectionKey, HConnectionImplementation> eldest)
{
>>> >>         return size() > MAX_CACHED_HBASE_INSTANCES;
>>> >>       }
>>> >>    };
>>> >>
>>> >>
>>> >> This only reads it from hbase-site.xml+hbase-default.xml. This is
>>> >> inconsistent, I think this should use ZKConfig.makeZKProps(conf) and
>>> then
>>> >> get the value.
>>> >>
>>> >> Thoughts?
>>> >>
>>> >> Lars
>>> >>
>>> >
>>>
>>

I would rather not break existing functionality. What happens when someone
downstream does a naive upgrade (it definitely happens, even though people
should read changelogs), and then their cluster goes down? Its probably
going to end up leading to a few hours searching stack traces, greping error
logs, and posting on the forums to track it down. I guess the severity of
this depends on how many people just distribute the zoo.cfg to all their
different applications, vs roll their confs in each application's
configuration as needed.

In the end, I would rather not break from existing functionality just
because it's functionality that some people don't use. As long as we iron
out the differences in the naming, its trivial to switch between the two,
but keeps people happy. Besides, no one says you have to use zoo.cfg ;)

-Jesse

On Wed, Jul 6, 2011 at 6:31 PM, Ryan Rawson <[EMAIL PROTECTED]> wrote:

> I was thinking that perhaps the normative use case for talking to a cluster
> is to specify the quorum name and path... The implicit config can be really
> confusing and is out of norms compared to other data store systems. Eg
> MySQL
> memcache etc.
> On Jul 6, 2011 2:14 PM, "Stack" <[EMAIL PROTECTED]> wrote:
> > I agree that we should be more consistent in how we get zk config
> > (Your original report looks like a bug Lars). I also recently tripped
> > over the fact that hbase uses different names for one or two zk
> > configs. We need to fix that too.
> > St.Ack
> >
> > On Mon, Jul 4, 2011 at 8:59 AM, Jesse Yates <[EMAIL PROTECTED]>
> wrote:
> >> Isn't that kind of the point though? If you drop in a zk config file on
> a
> >> machine, you should be able to update all your apps on that machine to
> the
> >> new config.
> >> Whats more important though is being able to easily distribute a changed
> zk
> >> config across your cluster and simultaneously across multiple
> applications.
> >> Rather than rewriting the confs for a handful of applications, and
> possibly
> >> making a mistake dealing with each application's own special semantics,
> and
> >> single conf to update everything just makes sense.
> >>
> >> I would lobby then that we make usage more consistent (as Lars
> recommends)
> >> and make some of the hbase conf values to more closely match the zk conf
> >> values (though hbase.${zk.value} is really not bad).
> >>
> >> -Jesse
> >>
> >>
> >> From: Ryan Rawson [[EMAIL PROTECTED]]
> >>> Sent: Monday, July 04, 2011 5:25 AM
> >>> To: [EMAIL PROTECTED]
> >>> Subject: Re: zoo.cfg vs hbase-site.xml
> >>>
> >>> Should just fully deprecate zoo.cfg, it ended up being more trouble
> >>> than it was worth.  When you use zoo.cfg you cannot connect to more
> >>> than 1 cluster from a single JVM.  Annoying!
> >>>
> >>> On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
> >>> > I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
> >>> > properties is different from the way we name
> >>> > "hbase.zookeeper.property.maxClientCnxns"
> >>> >
> >>> > e.g.
> >>> > # the port at which the clients will connect
> >>> > clientPort=2181
> >>> >
> >>> > FYI
> >>> >
> >>> > On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]>
> >>> wrote:
> >>> >
> >>> >> Hi,
> >>> >>
> >>> >> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
> >>> >> (including the ones from hbase-default.xml) - when present. But in
> some
> >>> >> places we do not consider this, for example in HConnectionManager:
> >>> >>
> >>> >>  static {
> >>> >>    // We set instances to one more than the value specified for
> {@link
> >>> >>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk
> default
> >>> >> max
> >>> >>    // connections to the ensemble from the one client is 30, so in
> that
> >>> >> case we
> >>> >>    // should run into zk issues before the LRU hit this value of 31.
> >>> >>    MAX_CACHED_HBASE_INSTANCES = HBaseConfiguration.create().getInt(
> >>> >>        HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS,

I made HBASE-4072 to address the issue Lars raises on top of this
thread and to address those areas where hbase uses other than the zk
attribute name for a zk property.
St.Ack

On Wed, Jul 6, 2011 at 8:04 PM, Jesse Yates <[EMAIL PROTECTED]> wrote:
> I would rather not break existing functionality. What happens when someone
> downstream does a naive upgrade (it definitely happens, even though people
> should read changelogs), and then their cluster goes down? Its probably
> going to end up leading to a few hours searching stack traces, greping error
> logs, and posting on the forums to track it down. I guess the severity of
> this depends on how many people just distribute the zoo.cfg to all their
> different applications, vs roll their confs in each application's
> configuration as needed.
>
> In the end, I would rather not break from existing functionality just
> because it's functionality that some people don't use. As long as we iron
> out the differences in the naming, its trivial to switch between the two,
> but keeps people happy. Besides, no one says you have to use zoo.cfg ;)
>
> -Jesse
>
> On Wed, Jul 6, 2011 at 6:31 PM, Ryan Rawson <[EMAIL PROTECTED]> wrote:
>
>> I was thinking that perhaps the normative use case for talking to a cluster
>> is to specify the quorum name and path... The implicit config can be really
>> confusing and is out of norms compared to other data store systems. Eg
>> MySQL
>> memcache etc.
>> On Jul 6, 2011 2:14 PM, "Stack" <[EMAIL PROTECTED]> wrote:
>> > I agree that we should be more consistent in how we get zk config
>> > (Your original report looks like a bug Lars). I also recently tripped
>> > over the fact that hbase uses different names for one or two zk
>> > configs. We need to fix that too.
>> > St.Ack
>> >
>> > On Mon, Jul 4, 2011 at 8:59 AM, Jesse Yates <[EMAIL PROTECTED]>
>> wrote:
>> >> Isn't that kind of the point though? If you drop in a zk config file on
>> a
>> >> machine, you should be able to update all your apps on that machine to
>> the
>> >> new config.
>> >> Whats more important though is being able to easily distribute a changed
>> zk
>> >> config across your cluster and simultaneously across multiple
>> applications.
>> >> Rather than rewriting the confs for a handful of applications, and
>> possibly
>> >> making a mistake dealing with each application's own special semantics,
>> and
>> >> single conf to update everything just makes sense.
>> >>
>> >> I would lobby then that we make usage more consistent (as Lars
>> recommends)
>> >> and make some of the hbase conf values to more closely match the zk conf
>> >> values (though hbase.${zk.value} is really not bad).
>> >>
>> >> -Jesse
>> >>
>> >>
>> >> From: Ryan Rawson [[EMAIL PROTECTED]]
>> >>> Sent: Monday, July 04, 2011 5:25 AM
>> >>> To: [EMAIL PROTECTED]
>> >>> Subject: Re: zoo.cfg vs hbase-site.xml
>> >>>
>> >>> Should just fully deprecate zoo.cfg, it ended up being more trouble
>> >>> than it was worth.  When you use zoo.cfg you cannot connect to more
>> >>> than 1 cluster from a single JVM.  Annoying!
>> >>>
>> >>> On Sun, Jul 3, 2011 at 10:22 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
>> >>> > I looked at conf/zoo_sample.cfg from zookeeper trunk. The naming of
>> >>> > properties is different from the way we name
>> >>> > "hbase.zookeeper.property.maxClientCnxns"
>> >>> >
>> >>> > e.g.
>> >>> > # the port at which the clients will connect
>> >>> > clientPort=2181
>> >>> >
>> >>> > FYI
>> >>> >
>> >>> > On Sun, Jul 3, 2011 at 9:53 AM, Lars George <[EMAIL PROTECTED]>
>> >>> wrote:
>> >>> >
>> >>> >> Hi,
>> >>> >>
>> >>> >> Usually the zoo.cfg overrides *all* settings off the hbase-site.xml
>> >>> >> (including the ones from hbase-default.xml) - when present. But in
>> some
>> >>> >> places we do not consider this, for example in HConnectionManager:
>> >>> >>
>> >>> >>  static {
>> >>> >>    // We set instances to one more than the value specified for
>> {@link
>> >>> >>    // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk