Mike- Just to add to your point about chaining methods in the context of held resources in the API, the following does not lend itself to be a very intuitive way to understand where and when connection resources can be cleaned and objects are no longer valid to use.
instance.getConnector(user, pass).getInstance().getConnector(user, pass).getInstance().getConnector(user, pass).getInstance()... -- Christopher L Tubbs II http://gravatar.com/ctubbsii On Mon, Dec 23, 2013 at 4:36 PM, Mike Drob <[email protected]> wrote: > Keith, > > I'm not sure I understand the alternative solution that you and Christopher > discussed. Can you explain it in a bit more detail, please? I have my own > interpretation of what I _think_ you were proposing, but I'd rather not > risk putting words in your mouth. Specifically, I'm interested in the > auto-cleanup aspect of things. > > I do think that the numbered changes you propose are good ideas. I also > agree with Christopher that we need to seriously examine the API that we > have, because chaining methods four- or five-deep [e.g. new > Instance().getConnector().tableOperations().createTable()] to do something > is not a great user experience. Determining the scope of the changes is > another community conversation. > > Mike > > > On Mon, Dec 23, 2013 at 9:17 AM, Keith Turner <[email protected]> wrote: > >> On Sun, Dec 22, 2013 at 10:28 PM, Christopher <[email protected]> wrote: >> >> > On Sun, Dec 22, 2013 at 2:23 PM, Bill Havanki <[email protected] >> > >> > wrote: >> > [snip] >> > > Although there was no intention of circumventing consensus, looking at >> > the >> > > email exchange, consensus was clearly not reached. >> > >> > It is my understanding that typically, in CtR, consensus is needed to >> > resolve issues after they are committed, where there is >> > conflict/objections. Perhaps it was my misunderstanding of the >> > responses, but it was my understanding that while there was no >> > consensus on the final solution, there was no objection that would >> > have prevented the interim action taken. >> > >> > > The short time span did >> > > not give others the chance to work on eliminating the warnings, as they >> > > offered, or to instead come around to just dropping Closeable. >> > >> > True... the timespan was short. My goal, as stated in the original >> > email, was to commit first (just like I might commit any improvement >> > to the current state of the code), and I intended the email to just be >> > an explanation of the reasoning, as it related to the prior commits, >> > and a prompt for discussion of further action. The fact that I >> > submitted the email chronologically first was a bit arbitrary. I >> > accept blame for the confusion of that, and any inciting wording the >> > email may have caused... I probably could have prepped things a bit >> > better... I have many personal "lessons learned" from this. :) >> > >> > > Personally, >> > > I am ambivalent about it. In any event, -1923 now exists to >> > comprehensively >> > > tackle the issue, and I eagerly welcome input and help on it. >> > > >> > > Removing Closeable did not undo all the work done, but it did undo some >> > of >> > > it. It's OK to call it that. Sometimes undoing is fine. That part of >> the >> > > commit for -2010 is a minimal change. We all agree Closeable should be >> > > there eventually, which is more important. We'll get it back. >> > >> > "undo" or "improve upon" is probably a semantic difference... but >> > yeah, my intent was to make it trivial to re-introduce if we decided >> > it was best to keep it. >> > >> > However, I'm not sure we all agree that Closeable should be there >> > eventually. I cannot speak for Keith Turner (hopefully, he'll chime in >> > >> >> Its not just Closeable, I am uncertain about adding a Instance.close() to >> the API. Its a very broad change to solve a very specific problem, which >> is not a problem in itself. My specific concern is that its a big change >> to the API w/o much vetting. I think the following changes should be made >> before release. >> >> 1) Modify existing examples to use the new instance.close() call. >> 2) Modify existing test to use the new instance.close() call. >> 3) Address the warnings >> 4) Create test to verify the expected behavior of the new instance.close() >> call. For example verify that Scanners, BatchScanner, tablet operations, >> etc all stop working when an instance is closed (if this is the intended >> behavior?). Verify that closing one instance object does not impact other >> instance objects. >> >> The purpose of #1, #2, and #3 is to eat our own dog food. Doing #1, #2, >> and #3 would determine what it will be like for users. This change will >> impact all users, its not a dark corner of the API its at the front door. >> If we release with Instance.close(), we will be stuck supporting it for >> years to come. I think it would be good to try to understand the >> implications of that the best we can now. #4 is needed to define the >> expected behavior of the new API and ensure its achievable. Also, I >> opened a bug about the new method not achieving its original goal in the >> case of multiple threads. >> >> As Christopher said, we have discussed alternative means of solving the web >> container problem that are more targeted. I would like prototype >> something, but I have not had time so far. Don't let my airing of >> concerns give the impression that I do not want to see a solution offered >> to users. If there is no alternative solution I have no desire to put up >> road blocks for the existing solution. This is an annoying problem for >> which users have no workaround and I really want to give users a way to >> address it. >> >> >> >> > at some point), but he and I have discussed this a bit, and I get the >> > distinct impression that he thinks it should not be there. >> > >> > > I never saw any compiler warnings because I don't use Eclipse. I can >> > > appreciate wanting to kill annoying warnings, but it would have been >> > better >> > > to tell Eclipse to STFU about them, until we could come around to >> > resolving >> > > them. If and when we do introduce some pertinent bylaws, the >> > peculiarities >> > > of an IDE should not drive them. Tools are there to help us, not tell >> us >> > > what to do. >> > >> > It's my understanding that these aren't Eclipse warnings, these are >> > default JDK1.6 compiler warnings. I could be wrong here... they may >> > need "javac -Xlint:all", or some other flag, to show up. In any case, >> > whether it is Eclipse, or FindBugs, or some other tool reporting >> > potential problems, I'm not concerned about them for aesthetics... I'm >> > concerned because they hint at potential areas of improvements or >> > bugs, that we should inspect with due diligence, and when they become >> > numerous, it's hard to actually tell the difference between a non-bug >> > warning that we've ignored and an actual bug warning that we've not >> > examined yet. >> > >> > In any case, the point is moot here, because even if it didn't produce >> > a warning, the current implementation does not warrant giving >> > incorrect information to the API consumer that it can/should be >> > closed, in accordance with Closeable's semantics (as in the case of >> > the currently broken MapReduce configuration code... See comment on >> > ACCUMULO-1923, which affects our code, and any subclasses of the >> > Input/OutputFormat). I would even go so far as to say that this >> > warning actually reflects an API bug: Instance does not actually >> > conform to Closeable's semantics... because it doesn't free resources >> > held by Instance... it frees static resources held elsewhere, and that >> > becomes obvious when we actually try to close it in accordance with >> > the semantics of Closeable, so it shouldn't be marked as such (until >> > we write the code to make it conform to those semantics). >> > >> > > There should be no committer norm of unilaterality. (OK, for the most >> > > obviously trivial of changes, but that's it.) Never mind whether this >> > case >> > > was unilateral: we can agree that a unilateral action has the chance to >> > > make others feel less valued and frustrated … even if the action is a >> > > beneficial one! Bylaws are a great way to avoid this, by setting ground >> > > rules. They can strike a balance, because we also do not want to be >> > > paralyzed by excessive multilaterality. >> > > >> > > This is all part of the maturing of a software project. We need to >> focus >> > on >> > > it. A healthy community around Accumulo is necessary for it to succeed. >> > > >> > > Thanks for reading! >> > > Bill >> > [snip] >> > >> > Granted, yes, absolutely, agreed, and so on :) >> > (to be clear, when I say "committer norms", I mean of the CtR type... >> > it's unilateral to a point, until an objection from review) >> > >> > -- >> > Christopher L Tubbs II >> > http://gravatar.com/ctubbsii >> > >>
