You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/11/04 20:42:55 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #1769: Minor cleanup in BulkImport
milleruntime opened a new pull request #1769:
URL: https://github.com/apache/accumulo/pull/1769
* Remove exceptions from BulkImport and ConcurrentKeyExtentCache
internal classes that were declared but never thrown
* Remove some unused code in findOverlappingTablets
* Shorten a few lines that use lambdas
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1769: Minor cleanup in BulkImport
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1769:
URL: https://github.com/apache/accumulo/pull/1769#discussion_r517642505
##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java
##########
@@ -302,35 +302,25 @@ public MLong(long i) {
}
public interface KeyExtentCache {
- KeyExtent lookup(Text row)
- throws AccumuloException, AccumuloSecurityException, TableNotFoundException;
+ KeyExtent lookup(Text row);
}
public static List<KeyExtent> findOverlappingTablets(KeyExtentCache extentCache,
- FileSKVIterator reader)
- throws IOException, AccumuloException, AccumuloSecurityException, TableNotFoundException {
-
- Text startRow = null;
- Text endRow = null;
+ FileSKVIterator reader) throws IOException {
List<KeyExtent> result = new ArrayList<>();
Collection<ByteSequence> columnFamilies = Collections.emptyList();
- Text row = startRow;
- if (row == null)
- row = new Text();
+ Text row = new Text();
while (true) {
- // log.debug(filename + " Seeking to row " + row);
reader.seek(new Range(row, null), columnFamilies, false);
if (!reader.hasTop()) {
- // log.debug(filename + " not found");
break;
}
row = reader.getTopKey().getRow();
KeyExtent extent = extentCache.lookup(row);
- // log.debug(filename + " found row " + row + " at location " + tabletLocation);
result.add(extent);
row = extent.endRow();
- if (row != null && (endRow == null || row.compareTo(endRow) < 0)) {
Review comment:
I think the method originally took `startRow` and `endRow` as parameters but an update made that not needed. This [update](https://github.com/apache/accumulo/commit/7ef140ec40c3768859b848350db8c6d6d20f7a56#diff-f36575975d4075ddd82d4963f08430c2ee6755baed4949b4e96e82b8eb7810baR266) changed the parameters.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1769: Minor cleanup in BulkImport
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1769:
URL: https://github.com/apache/accumulo/pull/1769#discussion_r517635050
##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java
##########
@@ -302,35 +302,25 @@ public MLong(long i) {
}
public interface KeyExtentCache {
- KeyExtent lookup(Text row)
- throws AccumuloException, AccumuloSecurityException, TableNotFoundException;
+ KeyExtent lookup(Text row);
}
public static List<KeyExtent> findOverlappingTablets(KeyExtentCache extentCache,
- FileSKVIterator reader)
- throws IOException, AccumuloException, AccumuloSecurityException, TableNotFoundException {
-
- Text startRow = null;
- Text endRow = null;
+ FileSKVIterator reader) throws IOException {
List<KeyExtent> result = new ArrayList<>();
Collection<ByteSequence> columnFamilies = Collections.emptyList();
- Text row = startRow;
- if (row == null)
- row = new Text();
+ Text row = new Text();
while (true) {
- // log.debug(filename + " Seeking to row " + row);
reader.seek(new Range(row, null), columnFamilies, false);
if (!reader.hasTop()) {
- // log.debug(filename + " not found");
break;
}
row = reader.getTopKey().getRow();
KeyExtent extent = extentCache.lookup(row);
- // log.debug(filename + " found row " + row + " at location " + tabletLocation);
result.add(extent);
row = extent.endRow();
- if (row != null && (endRow == null || row.compareTo(endRow) < 0)) {
Review comment:
I can't tell what this was trying to do, but clearly your change is equivalent to existing behavior if `endRow` was never assigned to anything other than `null`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #1769: Minor cleanup in BulkImport
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #1769:
URL: https://github.com/apache/accumulo/pull/1769
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org