accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher <ctubb...@apache.org>
Subject Re: ACCUMULO-3177 and ACCUMULO-3178
Date Fri, 05 Dec 2014 16:13:50 GMT
Sean, I understand if you don't have time to review these now... would you
mind doing so in a follow-up issue, or by exercising a revert if you see an
issue with them? I think it's reasonable to go ahead and push these changes
now.


--
Christopher L Tubbs II
http://gravatar.com/ctubbsii

On Thu, Dec 4, 2014 at 5:47 PM, Christopher <ctubbsii@apache.org> wrote:

> Last update on this thread was over a week ago. Jenna was kind enough to
> squash in my reviews (which were essentially provided to her as a patch on
> top of her patch via GitHub). She has updated the ReviewBoard entries, and
> I have commented with my +1.
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
> On Wed, Nov 26, 2014 at 4:12 PM, Christopher <ctubbsii@apache.org> wrote:
>
>> On Wed, Nov 26, 2014 at 3:07 PM, Sean Busbey <busbey@cloudera.com> wrote:
>>
>>> I will be as timely as I can.
>>>
>>> Thanks. However, please understand that if you are unable to respond
>> timely, progress can/should/will proceed without you. I just want to make
>> sure you have opportunity.
>>
>>
>>> Please keep in mind that our community is volunteer based, people have a
>>> multitude of commitments, and it is a holiday week.
>>>
>>>
>> Sure, I just want to make sure you have ample time to review and we can
>> address any objections you may have (if you still have any), but I want
>> some time constraint, so these contributions don't sit idle indefinitely
>> (that's not fair to the contributor, or to those who'd like to leverage
>> these features). As long as the discussion is active (holidays/weekends
>> considered, of course), I will not attempt to push.
>>
>>
>>> I had presumed more feedback wasn't pressing since no one had mentioned
>>> on
>>> the tickets how my original concerns were addressed by the revamps. If
>>> someone could give me a summary on each it would make things go much
>>> faster.
>>>
>>>
>> Your feedback is pressing because you're the only one who expressed
>> concerns, and I want to make sure you have ample opportunity to be
>> involved. But, I don't want it to wait forever. I did believe I had
>> responded sufficiently to your questions/concerns... but if there's still
>> something outstanding that I have not answered or addressed, please raise
>> it again, explicitly, and in the context of the proposed changes, so I can
>> respond to it.
>>
>> Sure, I can give you a summary as I understand it:
>>
>> Overview of the issues:
>>
>> ACCUMULO-3177: Adds a per-table VolumeChooser to replace the
>> RandomVolumeChooser as the default VolumeChooser, with the default
>> per-table implementation (a new per-table property is added for this) is
>> now the RandomVolumeChooser. This is modeled after the per-table balancer.
>> The intent is to enable balancing decisions for a tablet's persistent
>> storage across volumes in the same way we allow balancing tablet's
>> "compute" operations across Tablet Servers. To do this, a
>> "VolumeChooserEnvironment" object is created and added to the chooser API,
>> which exposes the environment to the chooser (such as the tableId, from
>> which the namespace can be retrieved, etc.)
>>
>> ACCUMULO-3178: Adds an additional example/reference implementation for a
>> per-table VolumeChooser, called a "PreferredVolumeChooser", which allows
>> users to specify a specific volume (or set of volumes) in a custom
>> per-table property. This is used as a test case (integration test) for
>> ACCUMULO-3177. (Note: your original concerns about deduplication of HDFS's
>> HSM effort expressed in ACCUMULO-3089 seem to apply only to this issue, not
>> to ACCUMULO-3177; however, my hope is that you'll see that this is
>> sufficiently unrelated to that, that your original objections do not apply.)
>>
>> Also, some background (from my perspective):
>>
>> Recall: As explained on ACCUMULO-3089, your original concerns were
>> addressed by splitting the original issue into separate tasks, to help
>> clarify the scope and intentions of that issue with more narrowly defined
>> changesets. You were requested to explain your objections specifically in
>> the context of the changesets on those tasks, because I believed you to
>> have misunderstood the scope of the issue (as I stated on ACCUMULO-3089),
>> and I hoped that doing so would help clarify where your objections were, in
>> relation to the proposed code changes. As far as I can tell, you did do
>> that, with your comments on the subsequent issues.
>>
>> You commented on ACCUMULO-3177, suggesting that it might be better to
>> apply the configuration property on a namespace instead of a table. I
>> responded to that comment with an explanation of how table and namespace
>> configurations work in this context. As such, I hold that your comment
>> represents a recommended "best practice" for table management
>> (specifically, limiting the granting of ALTER_TABLE), but not an objection
>> to the table/namespace configuration-based implementation/addition itself.
>>
>> You have also commented on ACCUMULO-3178, with an indication that you did
>> not feel that the VolumeChooser interface itself was strictly "public API",
>> which I took to mean that you'd be more flexible in tolerating changes to
>> that API with a higher degree of risk. I did not see an objection to
>> ACCUMULO-3178, but it does depend on ACCUMULO-3177. You also suggested not
>> making these dependent on ACCUMULO-3176 (because of the presence of a
>> limited workaround by adding configuration to a namespace first), a request
>> which was accommodated (but ultimately unnecessary if ACCUMULO-3176 is
>> permitted) in a separate review which you did not comment on after
>> requesting it to be made.
>>
>>
>>
>>
>>
>>> --
>>> Sean
>>> On Nov 26, 2014 12:27 PM, "Christopher" <ctubbsii@apache.org> wrote:
>>>
>>> > Thank you for your response. I request that you please be timely, as
>>> these
>>> > issues have sat idle for a long time, and the maintenance burden of
>>> keeping
>>> > them current with the HEAD of master is becoming excessive. I await
>>> your
>>> > reviews/objections. Thanks.
>>> >
>>> >
>>> > --
>>> > Christopher L Tubbs II
>>> > http://gravatar.com/ctubbsii
>>> >
>>> > On Wed, Nov 26, 2014 at 11:30 AM, Sean Busbey <busbey@cloudera.com>
>>> wrote:
>>> >
>>> > > I will follow up on both of those tickets with my objections, please
>>> do
>>> > not
>>> > > apply them.
>>> > >
>>> > > On Tue, Nov 25, 2014 at 2:08 PM, Christopher <ctubbsii@apache.org>
>>> > wrote:
>>> > >
>>> > > > My intention is to, sometime this week, take a thorough look at
the
>>> > > patches
>>> > > > and reviews available for ACCUMULO-3177 (create a per-table
>>> > > VolumeChooser)
>>> > > > and ACCUMULO-3178 (example implementation of an alternate
>>> > > > VolumeChooser/test case for ACCUMULO-3177). Given the discussions
>>> > > > surrounding ACCUMULO-3176, and because they originated as the
>>> overall
>>> > > > ACCUMULO-3089 (which had some objections that may not have been
>>> > > resolved) I
>>> > > > wanted to give this notice, to ensure that there is opportunity
for
>>> > > > objections to occur and be discussed here first.
>>> > > >
>>> > > > Presumably, the objections for ACCUMULO-3176 are different from
any
>>> > that
>>> > > > might be held for 3177 and 3178, but there was limited follow-up
>>> > > > clarification of the objections raised after the issues were
>>> re-scoped
>>> > as
>>> > > > separate, more specific, JIRA sub-tasks. So, I'm not sure there
>>> are any
>>> > > > still outstanding for those. Since the reviews are still open
for
>>> > those,
>>> > > I
>>> > > > just wanted to invite discussion either here, or (perhaps even
>>> better)
>>> > in
>>> > > > the specific review in ReviewBoard.
>>> > > >
>>> > > > --
>>> > > > Christopher L Tubbs II
>>> > > > http://gravatar.com/ctubbsii
>>> > > >
>>> > >
>>> > >
>>> > >
>>> > > --
>>> > > Sean
>>> > >
>>> >
>>>
>>
>>
>

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