foundationdb icon indicating copy to clipboard operation
foundationdb copied to clipboard

[Java] RangeQueries do additional unnecessary native Range calls

Open scottfines opened this issue 4 years ago • 6 comments

Looking at running benchmarks, I began counting calls to getRange_internal in FDBTransaction.java, and noticed that, in a specific circumstance, it's possible to issue two native requests in order to fetch a single row.

Specifically, consider the following simple test:

  @Test
    public void onlyDoesOneNativeFetchForLargeKey() throws Exception{
        FDB fdb = FDB.selectApiVersion(700);
        Random rand = new Random();
        byte[] key= new byte[512];
        byte[] value = new byte[512];
        rand.nextBytes(key);
        rand.nextBytes(value);

        try(Database db = fdb.open()){
            db.run(tr->{
                tr.set(key,value);
            });
            db.run(tr ->{
               final AtomicLong internalCallCounter = new AtomicLong(0L);
                //getPtrDirect() is a temporary function put in FDBTransaction to make it possible to do this delegation easily
              FDBTransaction txn = new FDBTransaction(((FDBTransaction) tr).getPtrDirect(), db, db.getExecutor()) {
              @Override
              protected FutureResults getRange_internal(KeySelector begin, KeySelector end, int rowLimit,
                        int targetBytes, int streamingMode, int iteration, boolean isSnapshot, boolean reverse) {
                    internalCallCounter.incrementAndGet();
                    return super.getRange_internal(begin, end, rowLimit, targetBytes, streamingMode, iteration,
                               isSnapshot, reverse);
                 }
             };
             AsyncIterator<KeyValue> kvs = txn.getRange(Range.startsWith(new byte[] { key[0] })).iterator();

             Assert.assertTrue("Did not return a record!", kvs.hasNext());
             KeyValue n = kvs.next();
             Assert.assertArrayEquals("Did not return a key correctly!", key, n.getKey());
             Assert.assertArrayEquals("Did not return the corect value!", value, n.getValue());

             // now check the counter to see how many fetches we did
            Assert.assertEquals("performed too many getRange_internal fetches!", 1, internalCallCounter);
            return null;
         });
    }
}

Running this test will fail because internalCallCounter will be 2, not 1. Some debugging and I found that the row is returned in the first requested chunk, but that more is true within the AsyncRangeIterator, so it issues another request. That second request is empty, and superfluous.

By putting some trace statements within NativeAPI.actor.cpp, I was able to track the more call to the block

GetKeyValuesReply _rep =
    wait(loadBalance(cx.getPtr(), beginServer.second, &StorageServerInterface::getKeyValues, req,
            TaskPriority::DefaultPromiseEndpoint, false,
            cx->enableLocalityLoadBalance ? &cx->queueModel : nullptr));

which I believe is the actual network call to the server for getKeyValues, but I'm not 100% sure of. At any rate, this call sets _rep.more = true on the first call.

If I am correct about the block call above, this means that more is being returned by the server during the range call, even though there is no more data to be returned.

In a perfect world, this additional network call would not be required.

scottfines avatar Feb 18 '21 19:02 scottfines