accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean Busbey <busbey+li...@cloudera.com>
Subject Re: [DISCUSS] API changes to provide resource cleanup
Date Thu, 02 Jan 2014 23:04:05 GMT
Excellent! it sounds like we have consensus.

Keith, I think you have volunteered for both the revert work and
reimplementing the PoC Hammer.

Do you want help in either of those cases? Help with a functional test,
docs?


On Thu, Jan 2, 2014 at 5:00 PM, William Slacum <
wilhelm.von.cloud@accumulo.net> wrote:

> Voting for the hammer/hacksawjimdugging. I like the concept of being to
> track resources and clean them up, but the back end code isn't designed to
> deal with an instance in the way we're trying to model it.
>
>
> On Thu, Jan 2, 2014 at 2:46 PM, Josh Elser <josh.elser@gmail.com> wrote:
>
> > Bill Slacum and I had talked about unexpected breakages in API for
> clients
> > and internal by modifying ZooKeeperInstance (I think I might have
> mentioned
> > it already on one of the tickets).
> >
> > Considering some of the other work that Mike has started on in regards to
> > making an easier-to-use client API, Bill and I mused over an
> > InstanceFactory notion which could wrap different Instance
> implementations
> > for the various deployment requirements. We could leave the current ZKI
> > (close to?) how it works now, lift the non thread-safe pieces to a common
> > parent, and create some sort of ThreadsafeZKI.
> >
> > Obviously this is very hand-wavy, but I'm definitely leery to changing
> the
> > default impl for something so prevalent as ZKI. Thinking about the
> problem
> > with a clean slate seems best to me.
> >
> >
> > On 1/2/14, 1:36 PM, Eric Newton wrote:
> >
> >> All of our current code treats the Instance like a simple record:
> >>
> >> * immutable, and therefore
> >> * thread-safe
> >> * provides several fields that describe an instance
> >>
> >> When I tried to add calls to close() in our own code, I found that our
> >> disregard for the lifetime of an instance was implicit, and probably is
> in
> >> all our user's code, too.
> >>
> >> I think if we want to do something like #1, we'll have to do so through
> a
> >> new API, and not by changing Instance, and then deprecate Instance.  The
> >> mental model is just completely different.
> >>
> >> -Eric
> >>
> >>
> >> On Thu, Jan 2, 2014 at 12:47 PM, Sean Busbey <busbey+lists@cloudera.com
> >
> >> wrote:
> >>
> >>  Hey Folks!
> >>>
> >>> We need to come to some conclusions on what we're going to do for
> >>> resource
> >>> clean up. I'll attempt to summarize the situation and various options.
> >>> If I
> >>> missed something from our myriad of tickets and mailing list threads,
> >>> please bring it up.
> >>>
> >>> Brief Background:
> >>>
> >>> The existing client APIs presume that a large amount of global state
> will
> >>> persist for the duration of a JVM instance. This is at odds with
> >>> lifecycle
> >>> management in application containers, where a JVM is very long lived
> and
> >>> user provided applications are stood up and torn down. We have reports
> of
> >>> this causing OOM on JBoss[1] and leaked threads on Tomcat[2].
> >>>
> >>> We have two possible solutions, both of which Jared Winick has kindly
> >>> verified solve the problem, as seen on JBoss.
> >>>
> >>> ----
> >>> = Proposed solution #1: Closeable Instance
> >>>
> >>> The first approach adds a .close method to Instance so that users can
> say
> >>> when they are done with a given instance. Internally, reference
> counting
> >>> determines when we tear down global resources.
> >>>
> >>> Advantages:
> >>>    * States via code where a client should do lifecycle management.
> >>>    * Allows shutting down just some of the resources used.
> >>>    * Is already in the code base.
> >>>
> >>> Disadvantages:
> >>>    * Since lifecycle is getting added post-hoc, we are more likely to
> >>> have
> >>> maintenance issues as we find other side effects we hadn't considered,
> >>> like
> >>> the multithreaded issue that already came up[3].
> >>>    * Changes Instance from representing static configuration to shared
> >>> state
> >>>    * Doesn't work with the fluent style some of our APIs encourage.
> >>>    * closed semantics probably aren't consistently enforced (e.g. users
> >>> trying to use a BatchWriter that came from a now-closed instance should
> >>> fail)
> >>>
> >>> To finish, we'd need to
> >>>    * Verify multithreaded handling is done without too much of a
> >>> performance
> >>> impact[3]
> >>>    * Finish making our internal use consistent with the lifecycle we're
> >>> telling others to use[4]
> >>>    * Possibly add tests to verify consistent enforcement of closing on
> >>> objects derived from Instance
> >>>
> >>> = Proposed solution #2: Global cleanup utility, aka The Hammer
> >>>
> >>> As a band-aid to allow for "unload resources" without making changes to
> >>> the
> >>> API we instead provide a utility method that cleans up all global
> >>> resources.
> >>>
> >>> Advantages:
> >>>    * Doesn't change API or meaning for Instance
> >>>    * Can be used on older Accumulo deployments w/o patch/rebuild cycle
> >>>
> >>> Disadvantages:
> >>>    * Only allows all-or-nothing cleanup
> >>>    * Doesn't address our underlying lack of lifecycle
> >>>    * Requires reverts
> >>>
> >>> To finish, we'd need to
> >>>    * revert commits from old solution (I haven't checked how many
> >>> commits,
> >>> but it's 6 tickets :/ )
> >>>    * port code from PoC to main codebase (asf grants, etc) [6]
> >>>    * add some kind of test (functional/IT?)
> >>>
> >>> -----
> >>>
> >>> We need to decide what we're going to provide as a placeholder for
> >>> releases
> >>> already frozen on API (i.e. 1.4, 1.5, 1.6*) as well as longer term.
> >>>
> >>> Personally, my position is that we should use the simplest change to
> >>> handle
> >>> the published versions (solution #2).
> >>>
> >>> Obviously there are outstanding issues with how we deal with global
> state
> >>> and shared resources in the current client APIs. I'd like to see that
> >>> addressed as a part of a more coherent client lifecycle rather than
> >>> struggling to make it work while maintaining the current API. Long
> term,
> >>> I
> >>> think this means handling things in the updated client API Christopher
> >>> has
> >>> mentioned a few times.
> >>>
> >>> How close to consensus are we already?
> >>>
> >>> - Sean
> >>>
> >>> [1]: https://issues.apache.org/jira/browse/ACCUMULO-1379
> >>> [2]: https://issues.apache.org/jira/browse/ACCUMULO-1697
> >>> [3]: https://issues.apache.org/jira/browse/ACCUMULO-2027
> >>> [4]: https://issues.apache.org/jira/browse/ACCUMULO-1923
> >>> [6]: https://issues.apache.org/jira/browse/ACCUMULO-2113
> >>>
> >>> *: I think 1.6 should be in this list because we are at feature freeze,
> >>> and
> >>> any proper changes to lifecycle management are likely to constitute an
> >>> addition that wouldn't pass that restriction. Mike Drob suggested in
> chat
> >>> that given the current state of 1.6 a more intrusive fix might be
> >>> acceptable.
> >>>
> >>>
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message