Recreate TabletsMetadata iterator when file ranges are not contiguous

[dlmarion]

In the Bulk Import v2 LoadFiles step a single TabletsMetadata object was used to map a tables tablets to a set of bulk import files. In the case where a small percentage of tablets were involved in the bulk import a majority of the tables tablets would still be evaluated. In the case where bulk imports were not importing into contiguous tablets the code would just iterate over the tables tablets until it found the next starting point.

This change recreates the TabletMetadata object when a set of files is not going to start at the next tablet in the table. A likely better way to achieve the same thing would be to reset the range on the underlying Scanner and create a new iterator, but the TabletsMetadata object does not expose the Scanner. This change also closes the TabletsMetadata objects which was not being done previously.

Related to #5201

[dlmarion]

Full IT build passed

[ddanielr]

Added some logging statements in #5345 to try and see how much time was being spent.

I was running BulkImportSequentialRowsIT directly just to generate some test code and the log shows that the tablet lookup generally took about 10ms - 14ms.

However, with these changes the timestamps jumped up to 64ms - 98ms which doesn't seem great.

Also found that the tablet skipping in findOverlappingTablets didn't seem to be getting called in the existing 2.1 code, but was getting called when these changes were applied.

   // skip tablets until we find the prevEndRow of loadRange
      while ((cmp = PREV_COMP.compare(currTablet.getPrevEndRow(), loadRange.prevEndRow())) < 0) {
        log.trace("{}: Skipping tablet: {}", fmtTid, currTablet.getExtent());
        currTablet = tabletIter.next();
      }

[ddanielr]

A new iterator is created each time when findOverlappingTablets is passed tm.iterator().

This causes the findOverlappingTablets method to attempt to start at the first row from the metadata builder range and skip ahead to the current range which adds significant delay.

To solve this we need to also track the iterator and assign a new one if the builder's range changes.

        tm = TabletsMetadata.builder(manager.getContext()).forTable(tableId)
            .overlapping(loadMapKey.prevEndRow(), null).checkConsistency()
            .fetch(PREV_ROW, LOCATION, LOADED).build();
        tabletIter = tm.iterator();
        
        List<TabletMetadata> tablets = findOverlappingTablets(fmtTid, loadMapKey, tabletIter);

[dlmarion]

Good info, I can look at this today.

[dlmarion]

To solve this we need to also track the iterator and assign a new one if the builder's range changes.

I just pushed this change. There is an issue in PrepBulkImport, it's not closing the TabletsMetadata object. Fixing it isn't straight forward, but I'm going to look at it too.

[ddanielr]

Ran Tests again and new changes are all coming back around 4-10ms.

[dlmarion]

I created #5355 to test tablet boundaries on the load files. I think that should be merged before this PR.

[keith-turner]

In some case this strategy could potentially make performance worse, like the case of importing into every 3rd tablet. The underlying scanner has already made an RPC and fetched some number of key values. Not sure of the best way to do this, but ideally we would only reset the scanner if the needed data is not already sitting in that batch of key values that was already read.

[keith-turner]

Wondering if using a batch scanner would be better here to minimize the overall number of RPCs made. Would be a large change to the code. The current code, even if we optimize the use of the scanner will make a lot of RPCs for some cases (like importing into every 100th tablet in a million tablet table) and those RPCs will be made serially. A batch scanner would minimize the number of RPCs made for these cases. Batch scanner could also make RPCs in parallel. The Scanner is probably more efficient for reading tablets sequentially, but probably not much slower than the batch scanner. I suspect the batch scanner would not be much slower when data is contiguous and would be much better when data is sparse.

Would be good to gather some performance data before making large changes to improve performance to ensure they are needed. Can not do it in 2.1, but in main we could experiment w/ the SplitMillionIT and try doing things like importing into every 10th tablet for 1000 tablets, every 100th tablet for 1000 tablets, etc.

[dlmarion]

Updated with recent changes in 2.1, to include the LoadFilesTest, which still passes.