You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tephra.apache.org by poornachandra <gi...@git.apache.org> on 2017/09/18 23:17:43 UTC
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
GitHub user poornachandra opened a pull request:
https://github.com/apache/incubator-tephra/pull/62
TEPHRA-245 Improve prune debug tool
JIRA - https://issues.apache.org/jira/browse/TEPHRA-245
Changes:
1. Improve documentation
2. Accept relative time as parameter
3. Print human readable dates
4. Sort the idle regions by prune upper bound
5. Fix warnings
6. Add test case
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/poornachandra/incubator-tephra feature/prune-debug-tool
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tephra/pull/62.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 #62
----
commit f62d6a2b490c706c1e6280a70cc5b3155eda239d
Author: poorna <po...@cask.co>
Date: 2017-09-14T00:08:29Z
TEPHRA-245 Add TimeMathParser to parse relative time
commit 6805f42f9c8ff25989e583e29a84fe33fd5ec051
Author: poorna <po...@cask.co>
Date: 2017-09-15T04:57:37Z
TEPHRA-245 Improve prune debug tool
1. Improve documentation
2. Accept relative time as parameter
3. Print human readable dates
4. Sort the idle regions by prune upper bound
5. Fix warnings
6. Add test case
----
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
Build is green, will squash and merge.
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
looks good. please apply this to the other compat modules.
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r139777982
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebug.java ---
@@ -56,15 +59,18 @@
public class InvalidListPruningDebug {
private static final Logger LOG = LoggerFactory.getLogger(InvalidListPruningDebug.class);
private static final Gson GSON = new Gson();
--- End diff --
Sure - will add that
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140183636
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTool.java ---
@@ -165,10 +165,10 @@ public void destroy() throws IOException {
*/
@Override
@SuppressWarnings("WeakerAccess")
- public SortedSet<RegionPruneInfoPretty> getIdleRegions(Integer numRegions, String time) throws IOException {
+ public List<RegionPruneInfoPretty> getIdleRegions(Integer numRegions, String time) throws IOException {
--- End diff --
👍
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tephra/pull/62
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
I trust you that you made the same changes in the other modules. LGTM
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
@anew I have ported the changes to other compat modules. Please take a look.
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r139811987
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebug.java ---
@@ -56,15 +59,18 @@
public class InvalidListPruningDebug {
private static final Logger LOG = LoggerFactory.getLogger(InvalidListPruningDebug.class);
private static final Gson GSON = new Gson();
--- End diff --
Ideally there is a command line option to control that, but actually people can pipe it through python. OK to leave as is.
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140172443
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTool.java ---
@@ -204,16 +204,19 @@ public int compare(RegionPruneInfo o1, RegionPruneInfo o2) {
return Long.compare(o1.getPruneUpperBound(), o2.getPruneUpperBound());
}
};
- Queue<RegionPruneInfoPretty> lowestPrunes =
+ MinMaxPriorityQueue<RegionPruneInfoPretty> lowestPrunes =
MinMaxPriorityQueue.orderedBy(comparator).maximumSize(numRegions).create();
for (RegionPruneInfo pruneInfo : regionPruneInfos) {
lowestPrunes.add(new RegionPruneInfoPretty(pruneInfo));
}
- TreeSet<RegionPruneInfoPretty> regionSet = new TreeSet<>(comparator);
- regionSet.addAll(lowestPrunes);
- return regionSet;
+ List<RegionPruneInfoPretty> regions = new ArrayList<>(numRegions);
+ RegionPruneInfoPretty e;
+ while ((e = lowestPrunes.pollFirst()) != null) {
--- End diff --
isn't that the same as addAll?
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140172872
--- Diff: tephra-hbase-compat-1.1-base/src/test/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTest.java ---
@@ -280,29 +282,36 @@ public void testIdleRegions() throws Exception {
try (PrintWriter out = new PrintWriter(outputStream)) {
// Get the list of regions that have the lowest prune upper bounds for the latest record time
Long latestRecordTime = compactedRegions.asMap().lastKey();
- SortedSet<InvalidListPruningDebugTool.RegionPruneInfoPretty> latestExpected =
- ImmutableSortedSet.copyOf(pruneUpperBoundComparator(), compactedRegions.get(latestRecordTime));
+ List<InvalidListPruningDebugTool.RegionPruneInfoPretty> latestExpected =
+ Lists.newArrayList(compactedRegions.get(latestRecordTime));
pruningDebug.execute(new String[]{"idle-regions", "-1"}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with explicit time
outputStream.reset();
pruningDebug.execute(new String[]{"idle-regions", "-1", String.valueOf(latestRecordTime)}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with relative time
outputStream.reset();
pruningDebug.execute(new String[]{"idle-regions", "-1", "now-2s"}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with reduced number of regions
outputStream.reset();
- pruningDebug.execute(new String[]{"idle-regions", "2", String.valueOf(latestRecordTime)}, out);
+ int limit = 2;
+ pruningDebug.execute(new String[]{"idle-regions", String.valueOf(limit), String.valueOf(latestRecordTime)}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(subset(latestExpected, 0, 2)), readOutputStream(outputStream));
+ List<RegionPruneInfo> actualPruneInfos = GSON.fromJson(readOutputStream(outputStream), PRUNE_INFO_LIST_TYPE);
+ Set<String> actualRegions = Sets.newHashSet(Iterables.transform(actualPruneInfos, PRUNE_INFO_TO_STRING));
+ // Just comparing sizes, since returned order can be different when prune upper bounds are the same
--- End diff --
you can make that deterministic, see my comment above
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r139744215
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebug.java ---
@@ -56,15 +59,18 @@
public class InvalidListPruningDebug {
private static final Logger LOG = LoggerFactory.getLogger(InvalidListPruningDebug.class);
private static final Gson GSON = new Gson();
--- End diff --
maybe better to use a pretty-printing Gson?
```
new GsonBuilder().setPrettyPrinting().create();
```
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
LGTM
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
LGTM
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
@anew I have ported over the changes to other comapt modules, please take a look
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140183652
--- Diff: tephra-hbase-compat-1.1-base/src/test/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTest.java ---
@@ -280,29 +282,36 @@ public void testIdleRegions() throws Exception {
try (PrintWriter out = new PrintWriter(outputStream)) {
// Get the list of regions that have the lowest prune upper bounds for the latest record time
Long latestRecordTime = compactedRegions.asMap().lastKey();
- SortedSet<InvalidListPruningDebugTool.RegionPruneInfoPretty> latestExpected =
- ImmutableSortedSet.copyOf(pruneUpperBoundComparator(), compactedRegions.get(latestRecordTime));
+ List<InvalidListPruningDebugTool.RegionPruneInfoPretty> latestExpected =
+ Lists.newArrayList(compactedRegions.get(latestRecordTime));
pruningDebug.execute(new String[]{"idle-regions", "-1"}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with explicit time
outputStream.reset();
pruningDebug.execute(new String[]{"idle-regions", "-1", String.valueOf(latestRecordTime)}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with relative time
outputStream.reset();
pruningDebug.execute(new String[]{"idle-regions", "-1", "now-2s"}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(latestExpected), readOutputStream(outputStream));
+ assertEquals(latestExpected, readOutputStream(outputStream));
// Same command with reduced number of regions
outputStream.reset();
- pruningDebug.execute(new String[]{"idle-regions", "2", String.valueOf(latestRecordTime)}, out);
+ int limit = 2;
+ pruningDebug.execute(new String[]{"idle-regions", String.valueOf(limit), String.valueOf(latestRecordTime)}, out);
out.flush();
- Assert.assertEquals(GSON.toJson(subset(latestExpected, 0, 2)), readOutputStream(outputStream));
+ List<RegionPruneInfo> actualPruneInfos = GSON.fromJson(readOutputStream(outputStream), PRUNE_INFO_LIST_TYPE);
+ Set<String> actualRegions = Sets.newHashSet(Iterables.transform(actualPruneInfos, PRUNE_INFO_TO_STRING));
+ // Just comparing sizes, since returned order can be different when prune upper bounds are the same
--- End diff --
Done
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
Please wait for tests to pass
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140172193
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTool.java ---
@@ -165,10 +165,10 @@ public void destroy() throws IOException {
*/
@Override
@SuppressWarnings("WeakerAccess")
- public SortedSet<RegionPruneInfoPretty> getIdleRegions(Integer numRegions, String time) throws IOException {
+ public List<RegionPruneInfoPretty> getIdleRegions(Integer numRegions, String time) throws IOException {
--- End diff --
so now the result won't be sorted?
Another option is to keep the sorted set, but make the region name part of the comparable. So that if two have the same prune bound, they still have different sort keys but end up adjacent to each other in the result. That would be a sorted result.
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by gokulavasan <gi...@git.apache.org>.
Github user gokulavasan commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
Porting LGTM 👍
---
[GitHub] incubator-tephra issue #62: TEPHRA-245 Improve prune debug tool
Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:
https://github.com/apache/incubator-tephra/pull/62
these commits look good. Please apply to all compat modules.
---
[GitHub] incubator-tephra pull request #62: TEPHRA-245 Improve prune debug tool
Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/62#discussion_r140183601
--- Diff: tephra-hbase-compat-1.1-base/src/main/java/org/apache/tephra/hbase/txprune/InvalidListPruningDebugTool.java ---
@@ -204,16 +204,19 @@ public int compare(RegionPruneInfo o1, RegionPruneInfo o2) {
return Long.compare(o1.getPruneUpperBound(), o2.getPruneUpperBound());
}
};
- Queue<RegionPruneInfoPretty> lowestPrunes =
+ MinMaxPriorityQueue<RegionPruneInfoPretty> lowestPrunes =
MinMaxPriorityQueue.orderedBy(comparator).maximumSize(numRegions).create();
for (RegionPruneInfo pruneInfo : regionPruneInfos) {
lowestPrunes.add(new RegionPruneInfoPretty(pruneInfo));
}
- TreeSet<RegionPruneInfoPretty> regionSet = new TreeSet<>(comparator);
- regionSet.addAll(lowestPrunes);
- return regionSet;
+ List<RegionPruneInfoPretty> regions = new ArrayList<>(numRegions);
+ RegionPruneInfoPretty e;
+ while ((e = lowestPrunes.pollFirst()) != null) {
--- End diff --
I noticed that `addAll()` can use a for loop to iterate over the elements of the priority queue, and adds the element to the destination collection.
The javadoc for the `MinMaxPriorityQueue.iterator()` says -
```
Returns an iterator over the elements contained in this collection, in no particular order
```
Hence `addAll()` may not be safe for preserving the order when adding to the list. But since we are moving back to `TreeSet`, we can use `addAll()` again.
---