You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by keith-turner <gi...@git.apache.org> on 2016/03/08 23:53:30 UTC
[GitHub] accumulo pull request: ACCUMULO-4112 use metadata table durability...
GitHub user keith-turner opened a pull request:
https://github.com/apache/accumulo/pull/79
ACCUMULO-4112 use metadata table durability config when writing minc …
…events to walog
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/keith-turner/accumulo ACCUMULO-4112
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/accumulo/pull/79.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #79
----
commit 714bb77b56f8b7177421908bed44c08bc0866b93
Author: Keith Turner <kt...@apache.org>
Date: 2016-03-08T15:19:27Z
ACCUMULO-4112 use metadata table durability config when writing minc events to walog
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request: ACCUMULO-4112 use metadata table durability...
Posted by wjsl <gi...@git.apache.org>.
Github user wjsl commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/79#discussion_r55471617
--- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java ---
@@ -2966,13 +2966,26 @@ public Void run() {
}
}
+ private Durability getMincEventDurability(KeyExtent extent) {
+ TableConfiguration conf;
+ if (extent.isMeta() || extent.isRootTablet()) {
--- End diff --
This check is a bit confusing. To me it reads "If the extent is for the metadata table or the root tablet, use the root tablet's configuration. Otherwise, use the metadata configuration." Is there some other setting for user tables that's more appropriate?
`KeyExtent#isMeta` already returns whether or not it's for the metadata table or the root tablet, so the `|| extent.isRootTablet()` is redundant. However, I think refactoring `KeyExtent#isMeta` is more appropriate to make it more clear about what it is checking.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request: ACCUMULO-4112 use metadata table durability...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner closed the pull request at:
https://github.com/apache/accumulo/pull/79
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request: ACCUMULO-4112 use metadata table durability...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/79#discussion_r55533890
--- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java ---
@@ -2966,13 +2966,26 @@ public Void run() {
}
}
+ private Durability getMincEventDurability(KeyExtent extent) {
+ TableConfiguration conf;
+ if (extent.isMeta() || extent.isRootTablet()) {
--- End diff --
> Is there some other setting for user tables that's more appropriate?
This is the best setting I know of... Since the walog minc entries are used during recovery, its nice to have them at the same sync level as the tablets metadata.
> so the || extent.isRootTablet() is redundant
Nice catch, I wrongly assumed isMeta() was a more narrow check. I used to know it did a wider check, but had forgotten.
> I think refactoring KeyExtent#isMeta is more appropriate
Changing its name would need to be a separate PR, because it would be very noisy drowning out the changes for 4112. I agree another name that implies more than `accumulo.metadata` would be good, however I am not sure what that name would be.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request: ACCUMULO-4112 use metadata table durability...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the pull request:
https://github.com/apache/accumulo/pull/79#issuecomment-194023887
:+1: pending some successful ITs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---