You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by kadirozde <gi...@git.apache.org> on 2018/10/30 16:33:09 UTC
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
GitHub user kadirozde opened a pull request:
https://github.com/apache/phoenix/pull/399
PHOENIX-4764 Cleanup metadata of child views for a base table that ha…
…s been dropped (addendum)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kadirozde/phoenix master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/399.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 #399
----
commit 960e47e88447da65ac4e3632e355b0c9070cfa18
Author: Kadir <ko...@...>
Date: 2018-10-30T04:14:33Z
PHOENIX-4764 Cleanup metadata of child views for a base table that has been dropped (addendum)
----
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229531588
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -70,7 +70,11 @@
private long timeMaxInterval = QueryServicesOptions.DEFAULT_TASK_HANDLING_MAX_INTERVAL_MS;
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
- private static final long initialDelay = 10000; // 10 secs
+ private static long initialDelay = 10000; // 10 secs
+
+ public static void disableTaskHandling() {
--- End diff --
Yes, that's what I realized later. I suggested this option as alternative but I guess we will be okay for now with this one.
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229504568
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -70,7 +70,11 @@
private long timeMaxInterval = QueryServicesOptions.DEFAULT_TASK_HANDLING_MAX_INTERVAL_MS;
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
- private static final long initialDelay = 10000; // 10 secs
+ private static long initialDelay = 10000; // 10 secs
+
+ public static void disableTaskHandling() {
--- End diff --
We should make this method "package-private".
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229417331
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -71,6 +71,11 @@
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
private static final long initialDelay = 10000; // 10 secs
+ private static boolean disableTaskHandling = false;
--- End diff --
You still have the `initialDelay` parameter, you could leverage it by exposing out as you did earlier and set it to `Integer.MAX_VALUE`. That approach is followed for the other rebuilder thread task.
OR
expose a parameter out that disables the complete framework, not just for test purposes. You could use set that parameter to false in tests, defaults to true.
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by kadirozde <gi...@git.apache.org>.
Github user kadirozde closed the pull request at:
https://github.com/apache/phoenix/pull/399
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by kadirozde <gi...@git.apache.org>.
Github user kadirozde commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229407775
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java ---
@@ -232,6 +232,7 @@ private boolean verifyMetricsFromSinkOnce(Map<String, Long> expectedMetrics) {
long actualValue = metric.value().longValue();
if (expectedValue != actualValue) {
LOG.warn("Metric from Hadoop Sink: " + metric.name() + " didn't match expected.");
+ LOG.warn("Expected: " + expectedValue + " Actual: " + actualValue);
--- End diff --
I will remove it.
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229528028
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -70,7 +70,11 @@
private long timeMaxInterval = QueryServicesOptions.DEFAULT_TASK_HANDLING_MAX_INTERVAL_MS;
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
- private static final long initialDelay = 10000; // 10 secs
+ private static long initialDelay = 10000; // 10 secs
+
+ public static void disableTaskHandling() {
--- End diff --
If its package-private then we won't be able to call it from the BasePhoenixMetricsIT.java right? Generally we usually have a config option to set parameters such as these (for example see QueryServices.INDEX_REBUILD_TASK_INITIAL_DELAY). Then you would put it in the server properties and call setUpTestDriver(ReadOnlyProps serverProps, ReadOnlyProps clientProps)
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by kadirozde <gi...@git.apache.org>.
Github user kadirozde commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229495572
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -71,6 +71,11 @@
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
private static final long initialDelay = 10000; // 10 secs
+ private static boolean disableTaskHandling = false;
--- End diff --
Will do it
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by kadirozde <gi...@git.apache.org>.
Github user kadirozde commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229546497
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TaskRegionObserver.java ---
@@ -70,7 +70,11 @@
private long timeMaxInterval = QueryServicesOptions.DEFAULT_TASK_HANDLING_MAX_INTERVAL_MS;
@GuardedBy("TaskRegionObserver.class")
// initial delay before the first task is handled
- private static final long initialDelay = 10000; // 10 secs
+ private static long initialDelay = 10000; // 10 secs
+
+ public static void disableTaskHandling() {
--- End diff --
OK. I have implemented the config option.
---
[GitHub] phoenix pull request #399: PHOENIX-4764 Cleanup metadata of child views for ...
Posted by karanmehta93 <gi...@git.apache.org>.
Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/399#discussion_r229401963
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java ---
@@ -232,6 +232,7 @@ private boolean verifyMetricsFromSinkOnce(Map<String, Long> expectedMetrics) {
long actualValue = metric.value().longValue();
if (expectedValue != actualValue) {
LOG.warn("Metric from Hadoop Sink: " + metric.name() + " didn't match expected.");
+ LOG.warn("Expected: " + expectedValue + " Actual: " + actualValue);
--- End diff --
You can remove this log line or merge it into previous one, otherwise it will be difficult to comprehend when all these log lines are together.
---