[GitHub] incubator-tephra pull request #3: Add logback-test.xml to reduce test output...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/3 Add logback-test.xml to reduce test output, and update dns provider for tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/fix-build Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/3.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 #3 commit 651337147ed80e502e33547eeadc785c7f1bb7ba Author: poorna Date: 2016-09-07T00:30:34Z Add logback-test.xml to reduce test output, and update dns provider for tests --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #3: Add logback-test.xml to reduce test output
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/3#discussion_r77885100
--- Diff: pom.xml ---
@@ -346,6 +352,18 @@
+
+org.slf4j
+log4j-over-slf4j
+${slf4j.version}
+test
--- End diff --
Since we don't know what kind of logging library will be used on the
cluster, we only change it for test scope.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #2: TEPHRA-179 Create a new instance of Transa...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/2#discussion_r77899348
--- Diff:
tephra-core/src/main/java/org/apache/tephra/runtime/TransactionDistributedModule.java
---
@@ -41,14 +41,15 @@
@Override
protected void configure() {
+// some of these classes need to be non-singleton in order to create a
new instance during leader() in
+// TransactionService
bind(SnapshotCodecProvider.class).in(Singleton.class);
-
bind(TransactionStateStorage.class).annotatedWith(Names.named("persist"))
- .to(HDFSTransactionStateStorage.class).in(Singleton.class);
-
bind(TransactionStateStorage.class).toProvider(TransactionStateStorageProvider.class).in(Singleton.class);
+
bind(TransactionStateStorage.class).annotatedWith(Names.named("persist")).to(HDFSTransactionStateStorage.class);
+
bind(TransactionStateStorage.class).toProvider(TransactionStateStorageProvider.class);
-bind(TransactionManager.class).in(Singleton.class);
-
bind(TransactionSystemClient.class).to(TransactionServiceClient.class).in(Singleton.class);
--- End diff --
Since `TransactionServiceClient` can contain pool of thrift clients, it is
better to have it as a singleton.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #2: TEPHRA-179 Create a new instance of Transa...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/2#discussion_r77899606
--- Diff:
tephra-core/src/main/java/org/apache/tephra/runtime/TransactionDistributedModule.java
---
@@ -41,14 +41,15 @@
@Override
protected void configure() {
+// some of these classes need to be non-singleton in order to create a
new instance during leader() in
+// TransactionService
bind(SnapshotCodecProvider.class).in(Singleton.class);
-
bind(TransactionStateStorage.class).annotatedWith(Names.named("persist"))
- .to(HDFSTransactionStateStorage.class).in(Singleton.class);
-
bind(TransactionStateStorage.class).toProvider(TransactionStateStorageProvider.class).in(Singleton.class);
+
bind(TransactionStateStorage.class).annotatedWith(Names.named("persist")).to(HDFSTransactionStateStorage.class);
+
bind(TransactionStateStorage.class).toProvider(TransactionStateStorageProvider.class);
-bind(TransactionManager.class).in(Singleton.class);
-
bind(TransactionSystemClient.class).to(TransactionServiceClient.class).in(Singleton.class);
-
bind(MetricsCollector.class).to(DefaultMetricsCollector.class).in(Singleton.class);
+bind(TransactionManager.class);
--- End diff --
This binding looks redundant now
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #2: TEPHRA-179 Create a new instance of Transa...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/2#discussion_r77901080
--- Diff:
tephra-core/src/main/java/org/apache/tephra/runtime/TransactionInMemoryModule.java
---
@@ -43,10 +44,12 @@ public TransactionInMemoryModule() {
@Override
protected void configure() {
-bind(SnapshotCodecProvider.class).in(Singleton.class);
-
bind(TransactionStateStorage.class).to(NoOpTransactionStateStorage.class).in(Singleton.class);
-bind(TransactionManager.class).in(Singleton.class);
-
bind(TransactionSystemClient.class).to(InMemoryTxSystemClient.class).in(Singleton.class);
+// some of these classes need to be non-singleton in order to create a
new instance during leader() in
+// TransactionService
+bind(SnapshotCodecProvider.class).in(Scopes.SINGLETON);
+
bind(TransactionStateStorage.class).to(NoOpTransactionStateStorage.class);
+bind(TransactionManager.class);
+bind(TransactionSystemClient.class).to(InMemoryTxSystemClient.class);
--- End diff --
This also needs to be singleton
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #2: TEPHRA-179 Create a new instance of Transa...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/2#discussion_r77906210
--- Diff:
tephra-core/src/test/java/org/apache/tephra/distributed/ThriftTransactionServerTest.java
---
@@ -188,21 +224,35 @@ public void process(WatchedEvent event) {
};
// Create another Zookeeper session with the same sessionId so that
the original one expires.
-final ZooKeeper dupZookeeper =
+ZooKeeper dupZookeeper =
new ZooKeeper(zkClientService.getConnectString(),
zooKeeper.getSessionTimeout(), watcher,
zooKeeper.getSessionId(),
zooKeeper.getSessionPasswd());
connectFuture.get(30, TimeUnit.SECONDS);
Assert.assertEquals("Failed to re-create current session",
dupZookeeper.getState(), ZooKeeper.States.CONNECTED);
dupZookeeper.close();
}
- private void waitForThriftTermination() throws InterruptedException {
-int count = 0;
-while (txService.thriftRPCServerState() != Service.State.TERMINATED &&
count++ < 200) {
+ private void waitFor(String errorMessage, Callable callable)
throws Exception {
+for (int i = 0; i < 200; i++) {
+ boolean value = callable.call();
+ if (value) {
+return;
+ }
TimeUnit.MILLISECONDS.sleep(50);
}
+Assert.fail(errorMessage);
+ }
+
+ private void waitForThriftStatus() throws Exception {
--- End diff --
Rename to `waitForThriftStop` for clarity?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #2: TEPHRA-179 Create a new instance of TransactionMa...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/2 @anwar6953 One minor comment, otherwise LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/4#discussion_r77928500 --- Diff: tephra-examples/cdh-5.8/pom.xml --- @@ -0,0 +1,96 @@ + + +http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + +tephra-examples +org.apache.tephra +0.9.0-incubating-SNAPSHOT + + 4.0.0 + + tephra-examples-cdh-5.8 + Apache Tephra Examples For CDH 5.8 + + + + cloudera + https://repository.cloudera.com/artifactory/cloudera-repos/ + + + + +2.6.0-cdh5.8.0 +1.2.0-cdh5.8.0 + + + +../src/main/java +../src/test/java + + + + + org.apache.tephra + tephra-hbase-compat-1.2-cdh --- End diff -- Now that I think about it, it would be better to add a separate compat modules for CDH 5.7 and CDH 5.8. Since we now avoid code duplication for compat modules, it is just one extra jar we need to publish. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/4#discussion_r77930303
--- Diff: tephra-examples/pom.xml ---
@@ -52,7 +64,7 @@
org.apache.hbase
hbase-common
- ${hbase12.version}
+ ${hbase11.version}
--- End diff --
HBase/Hadoop dependencies can be removed from the parent pom since the
child poms will redefine them anyway, right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/4#discussion_r77930451 --- Diff: tephra-core/src/test/resources/logback-test.xml --- @@ -30,7 +30,7 @@ - + --- End diff -- This change needs to be reverted. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/4#discussion_r77930996
--- Diff: tephra-hbase-compat-1.1-base/pom.xml ---
@@ -119,6 +121,12 @@
${hbase11.version}
--- End diff --
Same here, we should remove HBase/Hadoop dependencies from this parent pom
too.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #4: TEPHRA-176 TEPHRA-177 : Adding maven modules for ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/4 @sagarkapare This changes will require update to documentation too for supported versions. That can be done in a separate PR. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/4#discussion_r77936361 --- Diff: tephra-examples/cdh-5.8/pom.xml --- @@ -0,0 +1,96 @@ + + +http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + +tephra-examples +org.apache.tephra +0.9.0-incubating-SNAPSHOT + + 4.0.0 + + tephra-examples-cdh-5.8 + Apache Tephra Examples For CDH 5.8 + + + + cloudera + https://repository.cloudera.com/artifactory/cloudera-repos/ + + + + +2.6.0-cdh5.8.0 +1.2.0-cdh5.8.0 + + + +../src/main/java +../src/test/java + + + + + org.apache.tephra + tephra-hbase-compat-1.2-cdh --- End diff -- On second thoughts, this is not required. Since the HBase version is the same between CDH 5.7 and 5.8. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/4#discussion_r77942502
--- Diff: tephra-hbase-compat-1.1-base/pom.xml ---
@@ -0,0 +1,87 @@
+
+
+
+http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+
+org.apache.tephra
+tephra
+0.9.0-incubating-SNAPSHOT
+
+ 4.0.0
+
+ tephra-hbase-compat-1.1-base
+ Apache Tephra HBase 1.1 Compatibility Base
+
+ pom
+
+tephra-hbase-compat-1.2-cdh
+tephra-hbase-compat-1.1
+tephra-hbase-compat-1.2
+
+
+
+
+ org.apache.tephra
+ tephra-api
+ ${project.version}
+
+
+ org.apache.tephra
+ tephra-core
+ ${project.version}
+
+
+ org.apache.hbase
+ hbase
+
+
+
+
+
+
+ org.apache.tephra
+ tephra-core
+ ${project.version}
+ test-jar
+ test
+
+
+ junit
+ junit
+
+
+ org.slf4j
+ log4j-over-slf4j
+
+
+ org.slf4j
+ jcl-over-slf4j
+
+
+
+
+
+ hbase1.2
+
+1.2.1
--- End diff --
This profile can be removed since we have a separate compat module for
HBase 1.2 now
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #4: TEPHRA-176 TEPHRA-177 : Adding maven modul...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/4#discussion_r77943146 --- Diff: tephra-hbase-compat-1.1-base/tephra-hbase-compat-1.2/pom.xml --- @@ -0,0 +1,118 @@ + + + +http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + +org.apache.tephra +tephra-hbase-compat-1.1-base +0.9.0-incubating-SNAPSHOT +../pom.xml + + 4.0.0 + + tephra-hbase-compat-1.2 + Apache Tephra HBase 1.2 Compatibility + + +2.4.0 --- End diff -- HBase 1.1 and 1.2 use 2.5.1 Hadoop version. It would be good to use the same version here, and for 1.1 compat module. We should also update the examples to use the same version. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #4: TEPHRA-176 TEPHRA-177 : Adding maven modules for ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/4 @sagarkapare Two more minor comments to address --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #4: TEPHRA-176 TEPHRA-177 : Adding maven modules for ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/4 LGTM. Please squash the commits. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #6: Prefer IPv4 stack while running tests to a...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/6 Prefer IPv4 stack while running tests to avoid HBase 1.0 startup falures in dual-stack nodes You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/fix-test Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/6.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 #6 commit 272b8d6c1ee89c666d452ee2c8ad216e64e8d124 Author: poorna Date: 2016-09-09T01:02:45Z Prefer IPv4 stack while running tests to avoid HBase 1.0 startup failures in dual-stack nodes --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #7: Make TransactionManager Singleton for in-memory a...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/7 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #5: Fix flaky tests: ThriftTransactionServerTest#test...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/5 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #8: Fix flaky test (PooledClientProviderTest)
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/8#discussion_r78137456
--- Diff:
tephra-core/src/test/java/org/apache/tephra/distributed/PooledClientProviderTest.java
---
@@ -137,15 +137,15 @@ public Boolean call() throws Exception {
//Now race to get MAX_CLIENT_COUNT+1 clients, exhausting the pool and
requesting 1 more.
-List> clientIds = new ArrayList>();
-CountDownLatch countDownLatch = new CountDownLatch(1);
+List> clientIds = new ArrayList<>();
+// we want to ensure that all clients have been exhausted before
releasing any
+CountDownLatch countDownLatch = new CountDownLatch(MAX_CLIENT_COUNT);
--- End diff --
Would be good to call this `clientDoneLatch`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #8: Fix flaky test (PooledClientProviderTest)
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/8#discussion_r78137526
--- Diff:
tephra-core/src/test/java/org/apache/tephra/distributed/PooledClientProviderTest.java
---
@@ -154,11 +154,10 @@ public Boolean call() throws Exception {
// now, try it again with, where each thread holds onto the client for
twice the client.obtain.timeout value.
--- End diff --
Comments are outdated now
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #8: Fix flaky test (PooledClientProviderTest)
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/8 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #9: Wait for Transaction Service to announce i...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/9 Wait for Transaction Service to announce itself to fix flaky tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/fix-test Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/9.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 #9 commit 8f2cd72dbb59865c4edd4f2ff7ae9ccf96895bc3 Author: poorna Date: 2016-09-09T09:26:00Z Adding HBase dependencies back to tephra-examples pom for IDE commit c4c74e98704b770b6f5d5806839acec4089aeccf Author: poorna Date: 2016-09-09T09:31:12Z Wait for Transaction Service to announce itself to fix flaky tests --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #9: Wait for Transaction Service to announce i...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/9#discussion_r78230677
--- Diff: tephra-examples/pom.xml ---
@@ -51,6 +55,53 @@
tephra-core
${project.version}
+
--- End diff --
Adding HBase dependencies back to tephra-examples pom for IDE
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #9: Wait for Transaction Service to announce i...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/9#discussion_r78230869
--- Diff:
tephra-core/src/test/java/org/apache/tephra/ThriftTransactionSystemTest.java ---
@@ -88,10 +89,14 @@ protected void configure() {
txClient = injector.getInstance(TransactionSystemClient.class);
try {
LOG.info("Starting transaction service");
+ storage.startAndWait();
--- End diff --
It was being stopped without being started, so for consistency I am
starting it here. However, start on this object is a no-op.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #10: TEPHRA-179 Transaction service high avail...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/10 TEPHRA-179 Transaction service high availability changes Restructuring the Transaction Service classes to allow for HA restart while binding Transaction Manager and other classes as singletons You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/tx-service-ha Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/10.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 #10 commit 4b2bfe6a6733440fa73c7f5e00ff499662387911 Author: poorna Date: 2016-09-09T23:44:22Z TEPHRA-179 Transaction service high availability changes commit 7efff83009675a512f39cf8b0e94d1c6a1cde20a Author: poorna Date: 2016-09-10T00:24:26Z Add HA test --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #11: Add methods to help testing Transaction M...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/11 Add methods to help testing Transaction Manager PR https://github.com/apache/incubator-tephra/pull/2 made Transaction Manager and other classes non-singleton. This PR adds methods to fetch Transaction Manager and Transaction State Storage used by Transaction Service. You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/fix-tests Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/11.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 #11 commit e62d2882ee10a4ddc838768e6c41b1228fd8c2b8 Author: poorna Date: 2016-09-13T01:25:49Z Add methods to help testing Transaction Manager --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #10: TEPHRA-179 Transaction service high avail...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/10#discussion_r78485992
--- Diff:
tephra-core/src/main/java/org/apache/tephra/runtime/DefaultTransactionManagerProvider.java
---
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.runtime;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.tephra.TransactionManager;
+import org.apache.twill.zookeeper.ZKClient;
+import org.apache.twill.zookeeper.ZKClientService;
+
+/**
+ * A provider for {@link TransactionManager} that provides a new instance
every time.
+ */
+public class DefaultTransactionManagerProvider implements
Provider {
+ private final Configuration conf;
+ private final ZKClientService zkClientService;
+
+ @Inject
+ public DefaultTransactionManagerProvider(Configuration conf,
ZKClientService zkClientService) {
+this.conf = conf;
+this.zkClientService = zkClientService;
+ }
+
+ @Override
+ public TransactionManager get() {
+// Create a new injector every time since Guice services cannot be
restarted TEPHRA-179
+Injector injector = Guice.createInjector(
--- End diff --
This will require more decoupling. We'll need to break the class hierarchy
into HA Transaction Service, Transaction Service and Transaction Manager. For
now I have added some get methods to help testing in PR
https://github.com/apache/incubator-tephra/pull/11
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #10: TEPHRA-179 Transaction service high avail...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/10#discussion_r78493478
--- Diff:
tephra-core/src/main/java/org/apache/tephra/runtime/DefaultTransactionManagerProvider.java
---
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.runtime;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.tephra.TransactionManager;
+import org.apache.twill.zookeeper.ZKClient;
+import org.apache.twill.zookeeper.ZKClientService;
+
+/**
+ * A provider for {@link TransactionManager} that provides a new instance
every time.
+ */
+public class DefaultTransactionManagerProvider implements
Provider {
+ private final Configuration conf;
+ private final ZKClientService zkClientService;
+
+ @Inject
+ public DefaultTransactionManagerProvider(Configuration conf,
ZKClientService zkClientService) {
+this.conf = conf;
+this.zkClientService = zkClientService;
+ }
+
+ @Override
+ public TransactionManager get() {
+// Create a new injector every time since Guice services cannot be
restarted TEPHRA-179
+Injector injector = Guice.createInjector(
--- End diff --
Filed JIRA https://issues.apache.org/jira/browse/TEPHRA-182 for follow-up
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #13: (Tephra-184) Remove unused constants
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/13 LGTM, thanks for the contribution! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #14: (TEPHRA-185) Add a way to pass a custom t...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/14#discussion_r79964736
--- Diff:
tephra-core/src/main/java/org/apache/tephra/TransactionContext.java ---
@@ -88,7 +88,27 @@ public boolean removeTransactionAware(TransactionAware
txAware) {
* TransactionAware
*/
public void start() throws TransactionFailureException {
-currentTx = txClient.startShort();
+txClient.startShort();
+staretAllTxAwares();
+ }
+
+ /**
+ * Starts a new transaction. Calling this will initiate a new
transaction using the {@link TransactionSystemClient},
+ * and pass the returned transaction to {@link
TransactionAware#startTx(Transaction)} for each registered
+ * TransactionAware. If an exception is encountered, the transaction
will be aborted and a
+ * {@code TransactionFailureException} wrapping the root cause will be
thrown.
+ *
+ * @param timeout the transaction timeout for the transaction
+ *
+ * @throws TransactionFailureException if an exception occurs starting
the transaction with any registered
+ * TransactionAware
+ */
+ public void start(int timeout) throws TransactionFailureException {
+txClient.startShort(timeout);
--- End diff --
Same here about assignment
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #14: (TEPHRA-185) Add a way to pass a custom t...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/14#discussion_r79964659
--- Diff:
tephra-core/src/main/java/org/apache/tephra/TransactionContext.java ---
@@ -88,7 +88,27 @@ public boolean removeTransactionAware(TransactionAware
txAware) {
* TransactionAware
*/
public void start() throws TransactionFailureException {
-currentTx = txClient.startShort();
+txClient.startShort();
--- End diff --
`currentTx` still needs to be assigned the return value of
`txClient.startShort()`, right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #14: (TEPHRA-185) Add a way to pass a custom t...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/14#discussion_r79964694
--- Diff:
tephra-core/src/main/java/org/apache/tephra/TransactionContext.java ---
@@ -88,7 +88,27 @@ public boolean removeTransactionAware(TransactionAware
txAware) {
* TransactionAware
*/
public void start() throws TransactionFailureException {
-currentTx = txClient.startShort();
+txClient.startShort();
+staretAllTxAwares();
+ }
+
+ /**
+ * Starts a new transaction. Calling this will initiate a new
transaction using the {@link TransactionSystemClient},
+ * and pass the returned transaction to {@link
TransactionAware#startTx(Transaction)} for each registered
+ * TransactionAware. If an exception is encountered, the transaction
will be aborted and a
+ * {@code TransactionFailureException} wrapping the root cause will be
thrown.
+ *
+ * @param timeout the transaction timeout for the transaction
+ *
+ * @throws TransactionFailureException if an exception occurs starting
the transaction with any registered
+ * TransactionAware
+ */
+ public void start(int timeout) throws TransactionFailureException {
+txClient.startShort(timeout);
+staretAllTxAwares();
+ }
+
+ private void staretAllTxAwares() {
--- End diff --
Typo in `staret`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #14: (TEPHRA-185) Add a way to pass a custom t...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/14#discussion_r79965483
--- Diff:
tephra-core/src/main/java/org/apache/tephra/TransactionContext.java ---
@@ -97,6 +127,7 @@ public void start() throws TransactionFailureException {
txAware.getTransactionAwareName(),
currentTx.getTransactionId());
LOG.warn(message, e);
txClient.abort(currentTx);
+currentTx = null;
--- End diff --
ð
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #17: (TEPHRA-188) Allow to configure a limit f...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/17#discussion_r82304628
--- Diff:
tephra-core/src/main/java/org/apache/tephra/TransactionManager.java ---
@@ -722,7 +725,10 @@ public Transaction startShort() {
* @param timeoutInSeconds the time out period in seconds.
*/
public Transaction startShort(int timeoutInSeconds) {
-Preconditions.checkArgument(timeoutInSeconds > 0, "timeout must be
positive but is %s", timeoutInSeconds);
+Preconditions.checkArgument(timeoutInSeconds > 0,
+"timeout must be positive but is %s",
timeoutInSeconds);
+Preconditions.checkArgument(timeoutInSeconds <= maxTimeout,
+"timeout must not exceed %s but is %s",
maxTimeout, timeoutInSeconds);
--- End diff --
It would be good to add the units here
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #17: (TEPHRA-188) Allow to configure a limit for the ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/17 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #18: [TEPHRA-194] Make startShort() throw Ille...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/18#discussion_r84387594
--- Diff:
tephra-core/src/test/java/org/apache/tephra/TransactionSystemTest.java ---
@@ -33,25 +33,33 @@
*/
public abstract class TransactionSystemTest {
- public static final byte[] C1 = new byte[] { 'c', '1' };
- public static final byte[] C2 = new byte[] { 'c', '2' };
- public static final byte[] C3 = new byte[] { 'c', '3' };
- public static final byte[] C4 = new byte[] { 'c', '4' };
+ private static final byte[] C1 = new byte[] { 'c', '1' };
+ private static final byte[] C2 = new byte[] { 'c', '2' };
+ private static final byte[] C3 = new byte[] { 'c', '3' };
+ private static final byte[] C4 = new byte[] { 'c', '4' };
protected abstract TransactionSystemClient getClient() throws Exception;
protected abstract TransactionStateStorage getStateStorage() throws
Exception;
- // Unfortunately, in-memory mode and thrift mode throw different
exceptions here
- @Test(expected = Exception.class)
+ @Test
public void testNegativeTimeout() throws Exception {
-getClient().startShort(-1);
+try {
+ getClient().startShort(-1);
+ Assert.fail("Expected illegal argument for negative timeout");
+} catch (IllegalArgumentException e) {
--- End diff --
What if some other exception is thrown? Would be good to use
`@Test(expected = IllegalArgumentException.class)`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #18: [TEPHRA-194] Make startShort() throw Ille...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/18#discussion_r84388540
--- Diff:
tephra-core/src/test/java/org/apache/tephra/ThriftTransactionSystemTest.java ---
@@ -124,4 +127,39 @@ protected TransactionSystemClient getClient() throws
Exception {
protected TransactionStateStorage getStateStorage() throws Exception {
return storage;
}
+
+ @Override
+ public void testNegativeTimeout() throws Exception {
+CountingRetryStrategyProvider.retries.set(0);
+super.testNegativeTimeout();
+Assert.assertEquals(0, CountingRetryStrategyProvider.retries.get());
--- End diff --
It would be good to add a comment on why the retries should be zero here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #18: [TEPHRA-194] Make startShort() throw Ille...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/18#discussion_r84387667
--- Diff:
tephra-core/src/test/java/org/apache/tephra/TransactionSystemTest.java ---
@@ -33,25 +33,33 @@
*/
public abstract class TransactionSystemTest {
- public static final byte[] C1 = new byte[] { 'c', '1' };
- public static final byte[] C2 = new byte[] { 'c', '2' };
- public static final byte[] C3 = new byte[] { 'c', '3' };
- public static final byte[] C4 = new byte[] { 'c', '4' };
+ private static final byte[] C1 = new byte[] { 'c', '1' };
+ private static final byte[] C2 = new byte[] { 'c', '2' };
+ private static final byte[] C3 = new byte[] { 'c', '3' };
+ private static final byte[] C4 = new byte[] { 'c', '4' };
protected abstract TransactionSystemClient getClient() throws Exception;
protected abstract TransactionStateStorage getStateStorage() throws
Exception;
- // Unfortunately, in-memory mode and thrift mode throw different
exceptions here
- @Test(expected = Exception.class)
+ @Test
public void testNegativeTimeout() throws Exception {
-getClient().startShort(-1);
+try {
+ getClient().startShort(-1);
+ Assert.fail("Expected illegal argument for negative timeout");
+} catch (IllegalArgumentException e) {
+ // expected
+}
}
- // Unfortunately, in-memory mode and thrift mode throw different
exceptions here
- @Test(expected = Exception.class)
+ @Test
public void testExcessiveTimeout() throws Exception {
-getClient().startShort((int) TimeUnit.DAYS.toSeconds(10));
+try {
+ getClient().startShort((int) TimeUnit.DAYS.toSeconds(10));
+ Assert.fail("Expected illegal argument for excessive timeout");
+} catch (IllegalArgumentException e) {
--- End diff --
Same here
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #18: [TEPHRA-194] Make startShort() throw Ille...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/18#discussion_r84386647
--- Diff:
tephra-core/src/main/java/org/apache/tephra/distributed/TransactionServiceThriftClient.java
---
@@ -112,7 +113,20 @@ public Transaction startShort() throws TException {
public Transaction startShort(int timeout) throws TException {
try {
- return
TransactionConverterUtils.unwrap(client.startShortTimeout(timeout));
+ return
TransactionConverterUtils.unwrap(client.startShortWithTimeout(timeout));
+} catch (TGenericException e) {
+ isValid.set(false);
+ // currently, we only expect IllegalArgumentException here, if the
timeout is invalid
+ IllegalArgumentException actual;
+ try {
+actual = (IllegalArgumentException)
+ Class.forName(e.getOriginalExceptionClass())
+.getConstructor(String.class)
+.newInstance(e.getMessage());
+ } catch (Throwable t) {
--- End diff --
Would be good to log `t` in trace over here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #18: [TEPHRA-194] Make startShort() throw IllegalArgu...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/18 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #19: Save compaction state for pruning invalid...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/19 Save compaction state for pruning invalid list JIRA - https://issues.apache.org/jira/browse/TEPHRA-35 Adds ability to save prune upper bound from the transaction snapshot used for compaction. Note that the first two commits are re-factoring existing tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/transaction-pruning Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/19.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 #19 commit 3075cb3cf1b2b52c8946f18e9adec21e8a90d589 Author: poorna Date: 2016-10-28T22:12:23Z Save compaction state for pruning invalid list commit be048335024fe03ec567090f0dc2c121d9bff08a Author: poorna Date: 2016-10-29T00:46:01Z Refactor existing test commit 40ab5259722e8e138524c81b90bac2a16d455d24 Author: poorna Date: 2016-11-01T03:59:23Z Refactor createTable to not add transaction co-processor by default --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #19: TEPHRA-35 Save compaction state for pruni...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/19#discussion_r86888243
--- Diff: tephra-core/src/main/java/org/apache/tephra/TxConstants.java ---
@@ -345,4 +345,14 @@
public static final byte CURRENT_VERSION = 3;
}
+ /**
+ * Configuration for data janitor
+ */
+ public static final class DataJanitor {
+public static final String PRUNE_ENABLE = "data.tx.prune.enable";
+public static final String PRUNE_STATE_TABLE =
"data.tx.prune.state.table";
+
+public static final boolean DEFAULT_PRUNE_ENABLE = false;
+public static final String DEFAULT_PRUNE_STATE_TABLE =
"default:data_tx_janitor_state";
--- End diff --
Good point, I removed the namespace from the default value. It can always
be overridden in the configuration
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #19: TEPHRA-35 Save compaction state for pruni...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/19#discussion_r86889731
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -149,4 +149,15 @@ private static long getMaxTTL(Map
ttlByFamily) {
public static boolean isPreExistingVersion(long version) {
return version < MAX_NON_TX_TIMESTAMP;
}
+
+ /**
+ * Returns the maximum transaction that can be removed from the invalid
list for the state represented by the given
+ * transaction.
+ */
+ public static long getPruneUpperBound(Transaction tx) {
+long maxInvalidTx =
+ tx.getInvalids().length > 0 ?
tx.getInvalids()[tx.getInvalids().length - 1] : Transaction.NO_TX_IN_PROGRESS;
+long firstInProgress = tx.getFirstInProgress();
+return Math.min(maxInvalidTx, firstInProgress - 1);
--- End diff --
Good catch, updated the code to use the current read pointer in such a case
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #19: TEPHRA-35 Save compaction state for pruni...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/19#discussion_r86889803 --- Diff: tephra-hbase-compat-1.1-base/pom.xml --- @@ -28,6 +28,11 @@ tephra-hbase-compat-1.1-base Apache Tephra HBase 1.1 Compatibility Base + --- End diff -- Yes - it is required to run the tests through IDE --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #19: TEPHRA-35 Save compaction state for pruning inva...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/19 @anew One way to handle the above issue is to define a time limit that defines a maximum duration a transaction can be used for writing. While doing data writes, we could add some checks in the co-processor to enforce this time limit. While pruning we can use this time limit to remove invalid transactions that are older than this time limit. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #19: TEPHRA-35 Save compaction state for pruning inva...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/19 @anew I'll add the maximum duration check as a separate PR. I'll file a JIRA for it. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/20 Compute global prune upper bound using compaction state of every region JIRA - https://issues.apache.org/jira/browse/TEPHRA-198 This PR uses the compaction state recorded by co-processors in https://github.com/apache/incubator-tephra/pull/19 to compute the global prune upper bound. TODO: add some more test cases to test DefaultDataJanitorPlugin. You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/transaction-pruning Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/20.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 #20 commit 0c4001d088b432bbe68a5251e89665930d710f04 Author: poorna Date: 2016-11-10T23:09:02Z Compute global prune upper bound using compaction state of every region --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/20#discussion_r90580295 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/janitor/DataJanitorState.java --- @@ -19,20 +19,46 @@ package org.apache.tephra.hbase.coprocessor.janitor; +import com.google.common.collect.Maps; +import com.google.common.primitives.Longs; +import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.util.Bytes; import java.io.IOException; +import java.util.Map; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeMap; +import java.util.TreeSet; +import javax.annotation.Nullable; /** * Persist data janitor state into an HBase table. --- End diff -- Added javadoc --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/20#discussion_r90765092
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/janitor/HBaseTransactionPruningPlugin.java
---
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.hbase.coprocessor.janitor;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tephra.TxConstants;
+import org.apache.tephra.hbase.coprocessor.TransactionProcessor;
+import org.apache.tephra.janitor.TransactionPruningPlugin;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+/**
+ * Default implementation of the {@link TransactionPruningPlugin} for
HBase.
+ *
+ * This plugin determines the prune upper bound for transactional HBase
tables that use
+ * coprocessor {@link TransactionProcessor}.
+ *
+ * State storage:
+ *
+ * This plugin expects the TransactionProcessor to save the prune upper
bound for invalid transactions
+ * after every major compaction of a region. Let's call this (region,
prune upper bound).
+ * In addition, the plugin also persists the following information on a
run at time t
+ *
+ *
+ * (t, set of regions): Set of transactional regions at time
t.
+ * Transactional regions are regions of the tables that have the
coprocessor TransactionProcessor
+ * attached to them.
+ *
+ *
+ * (t, prune upper bound): This is the smallest not in-progress
transaction that
+ * will not have writes in any HBase regions that are created after
time t.
+ * This value is determined by the Transaction Service based on the
transaction state at time t
+ * and passed on to the plugin.
+ *
+ *
+ *
+ * Computing prune upper bound:
+ *
+ * In a typical HBase instance, there can be a constant change in the
number of regions due to region creations,
+ * splits and merges. At any given time there can always be a region on
which a major compaction has not been run.
+ * Since the prune upper bound will get recorded for a region only after a
major compaction,
+ * using only the latest set of regions we may not be able to find the
+ * prune upper bounds for all the current regions. Hence we persist the
set of regions that exist at that time
+ * of each run of the plugin, and use historical region set for time
t, t - 1, etc.
+ * to determine the prune upper bound.
+ *
+ * From the regions saved at time t, t - 1, etc.,
+ * the plugin tries to find the latest (t, set of regions) where
all regions have been major compacted,
+ * i.e, all regions have prune upper bound recorded in (region, prune
upper bound).
+ *
+ * If such a set is found for time t1, the prune upper bound
returned by the plugin is the minimum of
+ *
+ * Prune upper bounds of regions in set (t1, set of
regions)
+ * Prune upper bound from (t1, prune upper bound)
+ *
+ *
+ *
+ * Above, when we find (t1, set of regions), there may a region
that was created after time t1,
+ * but has a data write from an invalid transaction that i
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/20#discussion_r90765102
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/janitor/HBaseTransactionPruningPlugin.java
---
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.hbase.coprocessor.janitor;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tephra.TxConstants;
+import org.apache.tephra.hbase.coprocessor.TransactionProcessor;
+import org.apache.tephra.janitor.TransactionPruningPlugin;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+/**
+ * Default implementation of the {@link TransactionPruningPlugin} for
HBase.
+ *
+ * This plugin determines the prune upper bound for transactional HBase
tables that use
+ * coprocessor {@link TransactionProcessor}.
+ *
+ * State storage:
+ *
+ * This plugin expects the TransactionProcessor to save the prune upper
bound for invalid transactions
+ * after every major compaction of a region. Let's call this (region,
prune upper bound).
+ * In addition, the plugin also persists the following information on a
run at time t
+ *
+ *
+ * (t, set of regions): Set of transactional regions at time
t.
+ * Transactional regions are regions of the tables that have the
coprocessor TransactionProcessor
+ * attached to them.
+ *
+ *
+ * (t, prune upper bound): This is the smallest not in-progress
transaction that
+ * will not have writes in any HBase regions that are created after
time t.
+ * This value is determined by the Transaction Service based on the
transaction state at time t
+ * and passed on to the plugin.
+ *
+ *
+ *
+ * Computing prune upper bound:
+ *
+ * In a typical HBase instance, there can be a constant change in the
number of regions due to region creations,
+ * splits and merges. At any given time there can always be a region on
which a major compaction has not been run.
+ * Since the prune upper bound will get recorded for a region only after a
major compaction,
+ * using only the latest set of regions we may not be able to find the
+ * prune upper bounds for all the current regions. Hence we persist the
set of regions that exist at that time
+ * of each run of the plugin, and use historical region set for time
t, t - 1, etc.
+ * to determine the prune upper bound.
+ *
+ * From the regions saved at time t, t - 1, etc.,
+ * the plugin tries to find the latest (t, set of regions) where
all regions have been major compacted,
+ * i.e, all regions have prune upper bound recorded in (region, prune
upper bound).
+ *
+ * If such a set is found for time t1, the prune upper bound
returned by the plugin is the minimum of
+ *
+ * Prune upper bounds of regions in set (t1, set of
regions)
+ * Prune upper bound from (t1, prune upper bound)
+ *
+ *
+ *
+ * Above, when we find (t1, set of regions), there may a region
that was created after time t1,
+ * but has a data write from an invalid transaction that i
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/20#discussion_r90766057
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/janitor/HBaseTransactionPruningPlugin.java
---
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.hbase.coprocessor.janitor;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tephra.TxConstants;
+import org.apache.tephra.hbase.coprocessor.TransactionProcessor;
+import org.apache.tephra.janitor.TransactionPruningPlugin;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+/**
+ * Default implementation of the {@link TransactionPruningPlugin} for
HBase.
+ *
+ * This plugin determines the prune upper bound for transactional HBase
tables that use
+ * coprocessor {@link TransactionProcessor}.
+ *
+ * State storage:
+ *
+ * This plugin expects the TransactionProcessor to save the prune upper
bound for invalid transactions
+ * after every major compaction of a region. Let's call this (region,
prune upper bound).
+ * In addition, the plugin also persists the following information on a
run at time t
+ *
+ *
+ * (t, set of regions): Set of transactional regions at time
t.
+ * Transactional regions are regions of the tables that have the
coprocessor TransactionProcessor
+ * attached to them.
+ *
+ *
+ * (t, prune upper bound): This is the smallest not in-progress
transaction that
+ * will not have writes in any HBase regions that are created after
time t.
+ * This value is determined by the Transaction Service based on the
transaction state at time t
+ * and passed on to the plugin.
+ *
+ *
+ *
+ * Computing prune upper bound:
+ *
+ * In a typical HBase instance, there can be a constant change in the
number of regions due to region creations,
+ * splits and merges. At any given time there can always be a region on
which a major compaction has not been run.
+ * Since the prune upper bound will get recorded for a region only after a
major compaction,
+ * using only the latest set of regions we may not be able to find the
+ * prune upper bounds for all the current regions. Hence we persist the
set of regions that exist at that time
+ * of each run of the plugin, and use historical region set for time
t, t - 1, etc.
+ * to determine the prune upper bound.
+ *
+ * From the regions saved at time t, t - 1, etc.,
+ * the plugin tries to find the latest (t, set of regions) where
all regions have been major compacted,
+ * i.e, all regions have prune upper bound recorded in (region, prune
upper bound).
+ *
+ * If such a set is found for time t1, the prune upper bound
returned by the plugin is the minimum of
+ *
+ * Prune upper bounds of regions in set (t1, set of
regions)
+ * Prune upper bound from (t1, prune upper bound)
+ *
+ *
+ *
+ * Above, when we find (t1, set of regions), there may a region
that was created after time t1,
+ * but has a data write from an invalid transaction that i
[GitHub] incubator-tephra issue #20: Compute global prune upper bound using compactio...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/20 Thanks for the review @anew. I've addressed the comments, please take another look. > Regarding correctness, my major concern is that the (min(max invalid), min(inProgress)-1) is not a safe bound for transaction that are guaranteed not to write any more. I have filed TEPHRA-199 to address this, I'll make the changes in the next PR. Do you have a suggestion for a better name for `pruneUpperBoundTime`? Basically this is an upper bound for transactions that are not active (i.e, can be still used for writing). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #20: Compute global prune upper bound using co...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/20#discussion_r90766611
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/janitor/HBaseTransactionPruningPlugin.java
---
@@ -259,6 +262,8 @@ private long computePruneUpperBound(TimeRegions
timeRegions) throws IOException
if (pruneUpperBoundForTime != -1) {
Long minPruneUpperBoundRegions =
Collections.min(pruneUpperBoundRegions.values());
return Math.min(pruneUpperBoundForTime,
minPruneUpperBoundRegions);
+} else {
+ LOG.debug("Ignoring invalid prune upper bound -1 for time {}",
time);
}
} else {
if (LOG.isDebugEnabled()) {
--- End diff --
Sure, will update the message
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #20: Compute global prune upper bound using compactio...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/20 @anew Please take a look at the last rename commit --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #21: TEPHRA-202 Bump up twill.version to 0.8.0
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/21 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/22 Transaction pruning service https://issues.apache.org/jira/browse/TEPHRA-203 https://issues.apache.org/jira/browse/TEPHRA-199 This PR has three things - * Define and enforce maximum transaction lifetime where transactions can be used for data writes * Add `TransactionPruningService` to periodically schedule transaction prune run * Rename package name janitor to txprune It would be easier to review the PRs independently. You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/prune-service Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/22.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 #22 commit 33a7b6ffa30c9cd35c30de31e3384202f41ddef4 Author: poorna Date: 2016-12-05T08:25:16Z Transaction maximum lifetime commit fa45f385f290882af3fd4b482fe95c06ae241085 Author: poorna Date: 2016-12-06T09:55:55Z Invalid transaction pruning service commit cc44080d7cca9be73ce413629b3767c48a2d2462 Author: poorna Date: 2016-12-06T21:10:56Z Rename package janitor to txprune --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/22#discussion_r91224133 --- Diff: tephra-core/src/main/java/org/apache/tephra/TxConstants.java --- @@ -158,6 +158,14 @@ * The default value for the transaction timeout limit, in seconds: unlimited. */ public static final int DEFAULT_TX_MAX_TIMEOUT = Integer.MAX_VALUE; +/** + * The maximum time in seconds that a transaction can be used for data writes. --- End diff -- Yes, this limit applies to all transactions. Will update the documentation. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/22#discussion_r91224201
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/TransactionAwareHTable.java
---
@@ -649,6 +649,7 @@ private Delete transactionalizeAction(Delete delete)
throws IOException {
txDelete.setAttribute(entry.getKey(), entry.getValue());
}
txDelete.setDurability(delete.getDurability());
+addToOperation(txDelete, tx);
--- End diff --
We were not adding transaction as part of delete operation before. To
enforce the maximum lifetime we now need the transaction as part of the delete
operation.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/22#discussion_r91224362
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -179,6 +187,13 @@ public void
preGetOp(ObserverContext e, Get get, L
}
@Override
+ public void prePut(ObserverContext e, Put
put, WALEdit edit, Durability durability)
+throws IOException {
+Transaction tx = getFromOperation(put);
+ensureValidTxLifetime(tx);
+ }
+
+ @Override
--- End diff --
Increment and other operations are non-transactional today. Tephra does not
support readless increments.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/22#discussion_r91888402
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -163,8 +167,22 @@ public static long getPruneUpperBound(Transaction tx) {
long maxInvalidTx =
tx.getInvalids().length > 0 ?
tx.getInvalids()[tx.getInvalids().length - 1] : Transaction.NO_TX_IN_PROGRESS;
+
+// An invalid transaction can be used up to its max lifetime for data
writes, hence we cannot prune an invalid
+// transaction until it exhausts its max lifetime.
+long elapsedTime = currentTimeMillis -
TxUtils.getTimestamp(maxInvalidTx);
+long invalidTxBound;
+if (elapsedTime > txMaxLifetimeMillis) {
+ // maxInvalidTx is past its max lifetime
+ invalidTxBound = maxInvalidTx;
+} else {
+ // Reduce the lifetime of maxInvalidTx, so that it cannot be used
for any more writes
+ long remainingTime = txMaxLifetimeMillis - elapsedTime;
+ invalidTxBound = maxInvalidTx - remainingTime *
TxConstants.MAX_TX_PER_MS - 1;
--- End diff --
Good point. Now that I think about it, we can further simplify the method
`TxUtils.getPruneUpperBound()` to simply return `(currentTimeMillis -
txMaxLifetimeMillis) * TxConstants.MAX_TX_PER_MS - 1`, irrespective of whether
there are in-progress transactions or not. Since no transaction (in-progress or
invalid) can be used for writes after `txMaxLifetimeMillis`.
What do you say?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/22#discussion_r93238207
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -163,8 +167,22 @@ public static long getPruneUpperBound(Transaction tx) {
long maxInvalidTx =
tx.getInvalids().length > 0 ?
tx.getInvalids()[tx.getInvalids().length - 1] : Transaction.NO_TX_IN_PROGRESS;
+
+// An invalid transaction can be used up to its max lifetime for data
writes, hence we cannot prune an invalid
+// transaction until it exhausts its max lifetime.
+long elapsedTime = currentTimeMillis -
TxUtils.getTimestamp(maxInvalidTx);
+long invalidTxBound;
+if (elapsedTime > txMaxLifetimeMillis) {
+ // maxInvalidTx is past its max lifetime
+ invalidTxBound = maxInvalidTx;
+} else {
+ // Reduce the lifetime of maxInvalidTx, so that it cannot be used
for any more writes
+ long remainingTime = txMaxLifetimeMillis - elapsedTime;
+ invalidTxBound = maxInvalidTx - remainingTime *
TxConstants.MAX_TX_PER_MS - 1;
--- End diff --
On further thought, we will need to compute inactive transaction bound
separately from prune upper bound.
Inactive transaction bound can be computed using just current time and tx
max lifetime as ` (currentTimeMillis - txMaxLifetimeMillis) *
TxConstants.MAX_TX_PER_MS - 1`.
However, prune upper bound is a record of what invalid data was removed
during a major compaction, and hence will need to use the invalid list and
in-progress list from the transaction snapshot used at the time of the major
compaction.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #22: Transaction pruning service
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/22#discussion_r93367714
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -163,8 +167,22 @@ public static long getPruneUpperBound(Transaction tx) {
long maxInvalidTx =
tx.getInvalids().length > 0 ?
tx.getInvalids()[tx.getInvalids().length - 1] : Transaction.NO_TX_IN_PROGRESS;
+
+// An invalid transaction can be used up to its max lifetime for data
writes, hence we cannot prune an invalid
+// transaction until it exhausts its max lifetime.
+long elapsedTime = currentTimeMillis -
TxUtils.getTimestamp(maxInvalidTx);
+long invalidTxBound;
+if (elapsedTime > txMaxLifetimeMillis) {
+ // maxInvalidTx is past its max lifetime
+ invalidTxBound = maxInvalidTx;
+} else {
+ // Reduce the lifetime of maxInvalidTx, so that it cannot be used
for any more writes
+ long remainingTime = txMaxLifetimeMillis - elapsedTime;
+ invalidTxBound = maxInvalidTx - remainingTime *
TxConstants.MAX_TX_PER_MS - 1;
--- End diff --
Yes
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #22: Transaction pruning service
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/22 @anew I have separated out the computation of prune upper bound and inactive tx bound as discussed, please take another look. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #25: TEPHRA-206 Port Invalid list pruning changes to ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/25 @gokulavasan https://github.com/apache/incubator-tephra/pull/22 is now merged, please include changes from that PR too --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #25: TEPHRA-206 Port Invalid list pruning changes to ...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/25 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #26: TEPHRA-208 Compaction removes rows incorrectly f...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/26 @twdsilva Some code changes have not been ported over to file tephra-hbase-compat-1.1-base/src/test/java/org/apache/tephra/hbase/TransactionAwareHTableTest.java, hence the tests are failing. For instance, HDFSTransactionStateStorage is not initialized and miniDFS cluster is not started in that file. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/28#discussion_r97401610
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -140,29 +141,25 @@ public void start(CoprocessorEnvironment e) throws
IOException {
ttlByFamily.put(columnDesc.getName(), ttl);
}
- this.allowEmptyValues =
env.getConfiguration().getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
-
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+ String allowEmptyValuesFromTableDesc =
tableDesc.getValue(TxConstants.ALLOW_EMPTY_VALUES_KEY);
+ this.allowEmptyValues = (allowEmptyValuesFromTableDesc != null) ?
Boolean.valueOf(allowEmptyValuesFromTableDesc) :
+
getConfiguration(env).getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+
this.readNonTxnData =
Boolean.valueOf(tableDesc.getValue(TxConstants.READ_NON_TX_DATA));
if (readNonTxnData) {
LOG.info("Reading pre-existing data enabled for table " +
tableDesc.getNameAsString());
}
this.txMaxLifetimeMillis =
-
TimeUnit.SECONDS.toMillis(env.getConfiguration().getInt(TxConstants.Manager.CFG_TX_MAX_LIFETIME,
-
TxConstants.Manager.DEFAULT_TX_MAX_LIFETIME));
-
- boolean pruneEnabled =
env.getConfiguration().getBoolean(TxConstants.TransactionPruning.PRUNE_ENABLE,
-
TxConstants.TransactionPruning.DEFAULT_PRUNE_ENABLE);
- if (pruneEnabled) {
-String pruneTable =
env.getConfiguration().get(TxConstants.TransactionPruning.PRUNE_STATE_TABLE,
-
TxConstants.TransactionPruning.DEFAULT_PRUNE_STATE_TABLE);
-compactionState = new CompactionState(env,
TableName.valueOf(pruneTable), txMaxLifetimeMillis);
-LOG.debug("Automatic invalid list pruning is enabled. Compaction
state will be recorded in table " +
-pruneTable);
- }
+
TimeUnit.SECONDS.toMillis(getConfiguration(env).getInt(TxConstants.Manager.CFG_TX_MAX_LIFETIME,
--- End diff --
Same here about lazy read
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/28#discussion_r97401510
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -140,29 +141,25 @@ public void start(CoprocessorEnvironment e) throws
IOException {
ttlByFamily.put(columnDesc.getName(), ttl);
}
- this.allowEmptyValues =
env.getConfiguration().getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
-
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+ String allowEmptyValuesFromTableDesc =
tableDesc.getValue(TxConstants.ALLOW_EMPTY_VALUES_KEY);
+ this.allowEmptyValues = (allowEmptyValuesFromTableDesc != null) ?
Boolean.valueOf(allowEmptyValuesFromTableDesc) :
+
getConfiguration(env).getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
--- End diff --
Shouldn't this property also be lazily read?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/28#discussion_r97401714
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -140,29 +141,25 @@ public void start(CoprocessorEnvironment e) throws
IOException {
ttlByFamily.put(columnDesc.getName(), ttl);
}
- this.allowEmptyValues =
env.getConfiguration().getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
-
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+ String allowEmptyValuesFromTableDesc =
tableDesc.getValue(TxConstants.ALLOW_EMPTY_VALUES_KEY);
+ this.allowEmptyValues = (allowEmptyValuesFromTableDesc != null) ?
Boolean.valueOf(allowEmptyValuesFromTableDesc) :
+
getConfiguration(env).getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+
this.readNonTxnData =
Boolean.valueOf(tableDesc.getValue(TxConstants.READ_NON_TX_DATA));
if (readNonTxnData) {
LOG.info("Reading pre-existing data enabled for table " +
tableDesc.getNameAsString());
}
this.txMaxLifetimeMillis =
-
TimeUnit.SECONDS.toMillis(env.getConfiguration().getInt(TxConstants.Manager.CFG_TX_MAX_LIFETIME,
-
TxConstants.Manager.DEFAULT_TX_MAX_LIFETIME));
-
- boolean pruneEnabled =
env.getConfiguration().getBoolean(TxConstants.TransactionPruning.PRUNE_ENABLE,
-
TxConstants.TransactionPruning.DEFAULT_PRUNE_ENABLE);
- if (pruneEnabled) {
-String pruneTable =
env.getConfiguration().get(TxConstants.TransactionPruning.PRUNE_STATE_TABLE,
-
TxConstants.TransactionPruning.DEFAULT_PRUNE_STATE_TABLE);
-compactionState = new CompactionState(env,
TableName.valueOf(pruneTable), txMaxLifetimeMillis);
-LOG.debug("Automatic invalid list pruning is enabled. Compaction
state will be recorded in table " +
-pruneTable);
- }
+
TimeUnit.SECONDS.toMillis(getConfiguration(env).getInt(TxConstants.Manager.CFG_TX_MAX_LIFETIME,
+
TxConstants.Manager.DEFAULT_TX_MAX_LIFETIME));
}
}
+ protected Configuration getConfiguration(CoprocessorEnvironment env) {
--- End diff --
It would be good to add comments on the semantics of this method
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/28#discussion_r97436585
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -140,29 +141,25 @@ public void start(CoprocessorEnvironment e) throws
IOException {
ttlByFamily.put(columnDesc.getName(), ttl);
}
- this.allowEmptyValues =
env.getConfiguration().getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
-
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+ String allowEmptyValuesFromTableDesc =
tableDesc.getValue(TxConstants.ALLOW_EMPTY_VALUES_KEY);
+ this.allowEmptyValues = (allowEmptyValuesFromTableDesc != null) ?
Boolean.valueOf(allowEmptyValuesFromTableDesc) :
+
getConfiguration(env).getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
--- End diff --
Looks like `allowEmptyValues` was not a table level property before. Any
reason why we made it into a table level property now?
> If we do it lazily, then we might get different results for a Get
operation based on whether we were able to read the Configuration successfully
or not.
If not then it can be initialized incorrectly during startup, and will
always be wrong, right? I think it should always be read lazily.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #26: TEPHRA-208 Compaction removes rows incorrectly f...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/26 @twdsilva The changes look good. However, we recently did a re-factoring of TransactionAwareHTableTest by moving mini cluster startup/shutdown to a base class AbstractHBaseTableTest. It would be good if non-Transaction Manager startup/shutdown code and configuration setup in this PR are moved to AbstractHBaseTableTest. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/28#discussion_r97699439 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java --- @@ -303,10 +305,22 @@ public InternalScanner preCompactScannerOpen(ObserverContext
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/28#discussion_r97702427 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java --- @@ -303,10 +305,22 @@ public InternalScanner preCompactScannerOpen(ObserverContext
[GitHub] incubator-tephra pull request #28: TEPHRA-210 Get table specific properties ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/28#discussion_r97917171
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java
---
@@ -140,29 +143,37 @@ public void start(CoprocessorEnvironment e) throws
IOException {
ttlByFamily.put(columnDesc.getName(), ttl);
}
- this.allowEmptyValues =
env.getConfiguration().getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
-
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT);
+ String allowEmptyValuesFromTableDesc =
tableDesc.getValue(TxConstants.ALLOW_EMPTY_VALUES_KEY);
+ Configuration conf = getConfiguration(env);
+ boolean allowEmptyValuesFromConfig = (conf != null) ?
+conf.getBoolean(TxConstants.ALLOW_EMPTY_VALUES_KEY,
TxConstants.ALLOW_EMPTY_VALUES_DEFAULT) :
+TxConstants.ALLOW_EMPTY_VALUES_DEFAULT;
+
+ // If the property is not present in the tableDescriptor, get it
from the Configuration
+ this.allowEmptyValues = (allowEmptyValuesFromTableDesc != null) ?
--- End diff --
It might help readability if we move reading `allowEmptyValues` into a
private method
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #28: TEPHRA-210 Get table specific properties from ta...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/28 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #28: TEPHRA-210 Get table specific properties from ta...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/28 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98128325
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/PruneUpperBoundWriter.java
---
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tephra.hbase.txprune;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Thread that will write the the prune upper bound
+ */
+public class PruneUpperBoundWriter {
+ private static final Logger LOG =
LoggerFactory.getLogger(PruneUpperBoundWriter.class);
+ private static final Long PRUNE_CHECK_FREQUENCY =
TimeUnit.MINUTES.toMillis(1);
+
+ private final DataJanitorState dataJanitorState;
+ private final ConcurrentMap pruneUpperBound;
--- End diff --
Since region id is constant for a co-processor instance, we can pass the
region id in the constructor and use an atomic reference here, right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/29#discussion_r98135660 --- Diff: tephra-core/src/main/java/org/apache/tephra/TxConstants.java --- @@ -369,6 +369,12 @@ * Interval in seconds to schedule prune run. */ public static final String PRUNE_INTERVAL = "data.tx.prune.interval"; + +/** + * Interval in seconds to schedule flush of prune table entries to store. + */ +public static final String PRUNE_FLUSH_INTERVAL = "data.tx.prune.flush.interval"; --- End diff -- Looks like this is not used anywhere. Also I don't think we need this --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98135416
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/CompactionState.java
---
@@ -75,18 +76,34 @@ public void record(CompactionRequest request, @Nullable
TransactionVisibilitySta
}
/**
+ * Stops the current {@link PruneUpperBoundWriter}.
+ */
+ public void stop() {
+if (pruneUpperBoundWriter != null) {
+ pruneUpperBoundWriter.stop();
+}
+ }
+
+ /**
* Persists the transaction state recorded by {@link
#record(CompactionRequest, TransactionVisibilityState)}.
* This method is called after the compaction has successfully completed.
*/
public void persist() {
if (pruneUpperBound != -1) {
- try {
-dataJanitorState.savePruneUpperBoundForRegion(regionName,
pruneUpperBound);
-LOG.debug(String.format("Saved prune upper bound %s for region
%s", pruneUpperBound, regionNameAsString));
- } catch (IOException e) {
-LOG.warn(String.format("Cannot record prune upper bound in table
%s after compacting region %s",
- stateTable, regionNameAsString), e);
- }
+ PruneUpperBoundWriter upperBoundWriter = getPruneUpperBoundWriter();
--- End diff --
There are redundant instances of class `PruneUpperBoundWriter` =>
`upperBoundWriter` and a`pruneUpperBoundWriter`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/29#discussion_r98127853 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/PruneUpperBoundWriter.java --- @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tephra.hbase.txprune; + +import org.slf4j.Logger; --- End diff -- Since this code will run in HBase co-processor, use apache commons logging. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #26: TEPHRA-208 Compaction removes rows incorrectly f...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/26 I think that is fine. LGTM. Thanks for the fix @twdsilva! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/29#discussion_r98779081 --- Diff: tephra-core/src/main/java/org/apache/tephra/TxConstants.java --- @@ -369,6 +369,12 @@ * Interval in seconds to schedule prune run. */ public static final String PRUNE_INTERVAL = "data.tx.prune.interval"; + +/** + * Interval in seconds to schedule flush of prune table entries to store. + */ +public static final String PRUNE_FLUSH_INTERVAL = "data.tx.prune.flush.interval"; --- End diff -- In the first commit, it was not being used anywhere. Since it is being used now, it is okay. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98780134
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/PruneUpperBoundWriter.java
---
@@ -77,7 +78,7 @@ public void run() {
try {
dataJanitorState.savePruneUpperBoundForRegion(regionName,
pruneUpperBound.get());
} catch (IOException ex) {
-LOG.warn("Cannot record prune upper bound in table after
compacting region", ex);
+LOG.warn("Cannot record prune upper bound in table after
compacting region " + ex.getMessage());
--- End diff --
We can still pass in the Throwable to the logger.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98786925
--- Diff:
tephra-hbase-compat-1.1-base/src/test/java/org/apache/tephra/hbase/txprune/InvalidListPruneTest.java
---
@@ -135,6 +138,15 @@ private void deletePruneStateTable() throws Exception {
}
}
+ private void truncatePruneStateTable() throws Exception {
+if (hBaseAdmin.tableExists(pruneStateTable)) {
+ if (hBaseAdmin.isTableEnabled(pruneStateTable)) {
+hBaseAdmin.disableTable(pruneStateTable);
+ }
+ hBaseAdmin.truncateTable(pruneStateTable, true);
--- End diff --
No need to re-enable the table here?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/29#discussion_r98782081 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java --- @@ -324,7 +326,10 @@ public InternalScanner preCompactScannerOpen(ObserverContext
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98786420
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/PruneUpperBoundWriter.java
---
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tephra.hbase.txprune;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.tephra.hbase.coprocessor.TransactionProcessor;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * Thread that will write the the prune upper bound
+ */
+public class PruneUpperBoundWriter {
+ private static final Log LOG =
LogFactory.getLog(TransactionProcessor.class);
+
+ private final DataJanitorState dataJanitorState;
+ private final byte[] regionName;
+ private final long pruneFlushInterval;
+ private final AtomicLong pruneUpperBound;
+ private final AtomicBoolean shouldFlush;
+
+ private Thread flushThread;
+ private long lastChecked;
+
+ public PruneUpperBoundWriter(DataJanitorState dataJanitorState, byte[]
regionName, long pruneFlushInterval) {
+this.dataJanitorState = dataJanitorState;
+this.regionName = regionName;
+this.pruneFlushInterval = pruneFlushInterval;
+this.pruneUpperBound = new AtomicLong();
+this.shouldFlush = new AtomicBoolean(false);
+startFlushThread();
+ }
+
+ public boolean isAlive() {
+return flushThread.isAlive();
+ }
+
+ public void persistPruneEntry(long pruneUpperBound) {
+this.pruneUpperBound.set(pruneUpperBound);
+this.shouldFlush.set(true);
+ }
+
+ public void stop() {
+if (flushThread != null) {
+ flushThread.interrupt();
+}
+ }
+
+ private void startFlushThread() {
+flushThread = new Thread("tephra-prune-upper-bound-writer") {
+ @Override
+ public void run() {
+while (!isInterrupted()) {
+ long now = System.currentTimeMillis();
+ if (now > (lastChecked + pruneFlushInterval)) {
+if (shouldFlush.compareAndSet(true, false)) {
+ // should flush data
+ try {
+dataJanitorState.savePruneUpperBoundForRegion(regionName,
pruneUpperBound.get());
+ } catch (IOException ex) {
+LOG.warn("Cannot record prune upper bound in table after
compacting region " + ex.getMessage());
--- End diff --
Pass the Throwable to the logger here too
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98780450
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/HBaseTransactionPruningPlugin.java
---
@@ -209,6 +212,31 @@ public void destroy() {
}
}
+ /**
+ * Create the prune state table given the {@link TableName} if the table
doesn't exist already.
+ *
+ * @param stateTable prune state table name
+ */
+ protected void createPruneTable(TableName stateTable) throws IOException
{
+try (Admin admin = this.connection.getAdmin()) {
+ HTableDescriptor htd = new HTableDescriptor(stateTable);
+ htd.addFamily(new
HColumnDescriptor(DataJanitorState.FAMILY).setMaxVersions(1));
+ admin.createTable(htd);
--- End diff --
Since this happens on all regions on every startup, I think it would be
better to check for existence first.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98786621
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/PruneUpperBoundWriter.java
---
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tephra.hbase.txprune;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.tephra.hbase.coprocessor.TransactionProcessor;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * Thread that will write the the prune upper bound
+ */
+public class PruneUpperBoundWriter {
+ private static final Log LOG =
LogFactory.getLog(TransactionProcessor.class);
+
+ private final DataJanitorState dataJanitorState;
+ private final byte[] regionName;
+ private final long pruneFlushInterval;
+ private final AtomicLong pruneUpperBound;
+ private final AtomicBoolean shouldFlush;
+
+ private Thread flushThread;
+ private long lastChecked;
+
+ public PruneUpperBoundWriter(DataJanitorState dataJanitorState, byte[]
regionName, long pruneFlushInterval) {
+this.dataJanitorState = dataJanitorState;
+this.regionName = regionName;
+this.pruneFlushInterval = pruneFlushInterval;
+this.pruneUpperBound = new AtomicLong();
+this.shouldFlush = new AtomicBoolean(false);
+startFlushThread();
+ }
+
+ public boolean isAlive() {
+return flushThread.isAlive();
+ }
+
+ public void persistPruneEntry(long pruneUpperBound) {
+this.pruneUpperBound.set(pruneUpperBound);
+this.shouldFlush.set(true);
+ }
+
+ public void stop() {
+if (flushThread != null) {
+ flushThread.interrupt();
+}
+ }
+
+ private void startFlushThread() {
+flushThread = new Thread("tephra-prune-upper-bound-writer") {
+ @Override
+ public void run() {
+while (!isInterrupted()) {
+ long now = System.currentTimeMillis();
+ if (now > (lastChecked + pruneFlushInterval)) {
+if (shouldFlush.compareAndSet(true, false)) {
+ // should flush data
+ try {
+dataJanitorState.savePruneUpperBoundForRegion(regionName,
pruneUpperBound.get());
+ } catch (IOException ex) {
+LOG.warn("Cannot record prune upper bound in table after
compacting region " + ex.getMessage());
--- End diff --
Also it would be good to have the region name and the state table name in
the exception message.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request: https://github.com/apache/incubator-tephra/pull/29#discussion_r98804704 --- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/coprocessor/TransactionProcessor.java --- @@ -324,9 +326,14 @@ public InternalScanner preCompactScannerOpen(ObserverContext
[GitHub] incubator-tephra pull request #29: TEPHRA-212 Perform writes to prune state ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/29#discussion_r98803604
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/HBaseTransactionPruningPlugin.java
---
@@ -219,13 +219,20 @@ public void destroy() {
*/
protected void createPruneTable(TableName stateTable) throws IOException
{
try (Admin admin = this.connection.getAdmin()) {
+ if (admin.tableExists(stateTable)) {
+LOG.debug("Not creating pruneStateTable {} since it already
exists.",
+ stateTable.getNameWithNamespaceInclAsString());
+return;
+ }
+
HTableDescriptor htd = new HTableDescriptor(stateTable);
htd.addFamily(new
HColumnDescriptor(DataJanitorState.FAMILY).setMaxVersions(1));
admin.createTable(htd);
LOG.info("Created pruneTable {}",
stateTable.getNameWithNamespaceInclAsString());
} catch (TableExistsException ex) {
- // Expected if the prune state table already exists
- LOG.debug("Not creating pruneStateTable since it already exists.
{}", ex.getMessage());
+ // Expected if the prune state table is being created at the same
time by another client
+ LOG.debug("Not creating pruneStateTable {} since it already exists.
{}",
+stateTable.getNameWithNamespaceInclAsString(),
ex.getMessage());
--- End diff --
Log the throwable
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tephra issue #29: TEPHRA-212 Perform writes to prune state asynchr...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/29 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #29: TEPHRA-212 Perform writes to prune state asynchr...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/29 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #30: Use getRegion instead of getRegionInfo for hbase...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/30 What about 0.96 module? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #30: Use getRegion instead of getRegionInfo for hbase...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/30 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra issue #31: (TEPHRA-214) Tool to debug the state and progres...
Github user poornachandra commented on the issue: https://github.com/apache/incubator-tephra/pull/31 @gokulavasan I took a pass over this, I have a few comments. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-tephra pull request #31: (TEPHRA-214) Tool to debug the state and ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/31#discussion_r99751633
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebug.java
---
@@ -0,0 +1,191 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.hbase.txprune;
+
+import com.google.common.collect.MinMaxPriorityQueue;
+import com.google.gson.Gson;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tephra.TxConstants;
+import org.apache.tephra.txprune.RegionPruneInfo;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.Comparator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Queue;
+import javax.annotation.Nullable;
+
+/**
+ * Invalid List Pruning Debug Tool.
+ */
+public class InvalidListPruningDebug {
+ private static final Logger LOG =
LoggerFactory.getLogger(InvalidListPruningDebug.class);
+ private static final Gson GSON = new Gson();
+ private DataJanitorState dataJanitorState;
+
+ /**
+ * Initialize the Invalid List Debug Tool.
+ * @param conf {@link Configuration}
+ * @throws IOException
+ */
+ public void initialize(final Configuration conf) throws IOException {
+LOG.info("InvalidListPruningDebugMain : initialize method called");
+final Connection connection = ConnectionFactory.createConnection(conf);
+dataJanitorState = new DataJanitorState(new
DataJanitorState.TableSupplier() {
+ @Override
+ public Table get() throws IOException {
+return connection.getTable(TableName.valueOf(
+ conf.get(TxConstants.TransactionPruning.PRUNE_STATE_TABLE,
+
TxConstants.TransactionPruning.DEFAULT_PRUNE_STATE_TABLE)));
+ }
+});
+ }
+
+ /**
+ * Return a list of RegionPruneInfo. These regions are the ones that
have the lowest prune upper bounds.
+ * If -1 is passed in, all the regions and their prune upper bound will
be returned.
+ *
+ * @param numRegions number of regions
+ * @return Map of region name and its prune upper bound
+ */
+ public Queue getIdleRegions(Integer numRegions) throws
IOException {
+List regionPruneInfos =
dataJanitorState.getPruneInfoForRegions(null);
+if (regionPruneInfos.isEmpty()) {
+ return new LinkedList<>();
+}
+
+if (numRegions < 0) {
+ numRegions = regionPruneInfos.size();
+}
+
+Queue lowestPrunes =
MinMaxPriorityQueue.orderedBy(new Comparator() {
+ @Override
+ public int compare(RegionPruneInfo o1, RegionPruneInfo o2) {
+return (int) (o1.getPruneUpperBound() - o2.getPruneUpperBound());
+ }
+}).maximumSize(numRegions).create();
+
+for (RegionPruneInfo pruneInfo : regionPruneInfos) {
+ lowestPrunes.add(pruneInfo);
+}
+return lowestPrunes;
+ }
+
+ /**
+ * Return the prune upper bound value of a given region. If no prune
upper bound has been written for this region yet,
+ * it will return a null.
+ *
+ * @param regionId region id
+ * @return {@link RegionPruneInfo} of the region
+ * @throws IOException if there are any errors while trying to fetch the
{@link RegionPruneInfo}
+ */
+ @Nullable
+ public RegionPruneInfo getRegionPruneInfo(String regionId) throws
IOException {
+return
dataJanitorState.getPruneInfoForRegion(Bytes.toBytesBinary(regionId));
+ }
+
+ /**
[GitHub] incubator-tephra pull request #31: (TEPHRA-214) Tool to debug the state and ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/31#discussion_r99750355
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebug.java
---
@@ -0,0 +1,191 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tephra.hbase.txprune;
+
+import com.google.common.collect.MinMaxPriorityQueue;
+import com.google.gson.Gson;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.tephra.TxConstants;
+import org.apache.tephra.txprune.RegionPruneInfo;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.Comparator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Queue;
+import javax.annotation.Nullable;
+
+/**
+ * Invalid List Pruning Debug Tool.
+ */
+public class InvalidListPruningDebug {
+ private static final Logger LOG =
LoggerFactory.getLogger(InvalidListPruningDebug.class);
+ private static final Gson GSON = new Gson();
+ private DataJanitorState dataJanitorState;
+
+ /**
+ * Initialize the Invalid List Debug Tool.
+ * @param conf {@link Configuration}
+ * @throws IOException
+ */
+ public void initialize(final Configuration conf) throws IOException {
+LOG.info("InvalidListPruningDebugMain : initialize method called");
+final Connection connection = ConnectionFactory.createConnection(conf);
+dataJanitorState = new DataJanitorState(new
DataJanitorState.TableSupplier() {
+ @Override
+ public Table get() throws IOException {
+return connection.getTable(TableName.valueOf(
+ conf.get(TxConstants.TransactionPruning.PRUNE_STATE_TABLE,
+
TxConstants.TransactionPruning.DEFAULT_PRUNE_STATE_TABLE)));
+ }
+});
+ }
+
+ /**
+ * Return a list of RegionPruneInfo. These regions are the ones that
have the lowest prune upper bounds.
+ * If -1 is passed in, all the regions and their prune upper bound will
be returned.
+ *
+ * @param numRegions number of regions
+ * @return Map of region name and its prune upper bound
+ */
+ public Queue getIdleRegions(Integer numRegions) throws
IOException {
+List regionPruneInfos =
dataJanitorState.getPruneInfoForRegions(null);
+if (regionPruneInfos.isEmpty()) {
+ return new LinkedList<>();
+}
+
+if (numRegions < 0) {
+ numRegions = regionPruneInfos.size();
+}
+
+Queue lowestPrunes =
MinMaxPriorityQueue.orderedBy(new Comparator() {
+ @Override
+ public int compare(RegionPruneInfo o1, RegionPruneInfo o2) {
+return (int) (o1.getPruneUpperBound() - o2.getPruneUpperBound());
+ }
+}).maximumSize(numRegions).create();
+
+for (RegionPruneInfo pruneInfo : regionPruneInfos) {
+ lowestPrunes.add(pruneInfo);
+}
+return lowestPrunes;
+ }
+
+ /**
+ * Return the prune upper bound value of a given region. If no prune
upper bound has been written for this region yet,
+ * it will return a null.
+ *
+ * @param regionId region id
+ * @return {@link RegionPruneInfo} of the region
+ * @throws IOException if there are any errors while trying to fetch the
{@link RegionPruneInfo}
+ */
+ @Nullable
+ public RegionPruneInfo getRegionPruneInfo(String regionId) throws
IOException {
+return
dataJanitorState.getPruneInfoForRegion(Bytes.toBytesBinary(regionId));
+ }
+
+ /**
[GitHub] incubator-tephra pull request #31: (TEPHRA-214) Tool to debug the state and ...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/31#discussion_r99759929
--- Diff:
tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/DataJanitorState.java
---
@@ -102,11 +107,29 @@ public void savePruneUpperBoundForRegion(byte[]
regionId, long pruneUpperBound)
* @throws IOException when not able to read the data from HBase
*/
public long getPruneUpperBoundForRegion(byte[] regionId) throws
IOException {
+RegionPruneInfo regionPruneInfo = getPruneInfoForRegion(regionId);
+return (regionPruneInfo == null) ? -1 :
regionPruneInfo.getCompactionTimestamp();
--- End diff --
This has to return the `regionPruneInfo.getPruneUpperBound()`, right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
