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 <[email protected]> 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 <[email protected]> wrote: > >> On Wed, Nov 26, 2014 at 3:07 PM, Sean Busbey <[email protected]> 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" <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> >>> > 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 >>> > > >>> > >>> >> >> >
