Address review comments for PR #1756

Use fdb_future_get_int64 for language bindings and get rid of using Version
with getApproximateSize API.

bindings/c/fdb_c.cpp

@@ -220,8 +220,8 @@ fdb_error_t fdb_future_get_version( FDBFuture* f, int64_t* out_version ) {
 }
 
 extern "C" DLLEXPORT
-fdb_error_t fdb_future_get_int64( FDBFuture* f, int64_t* out ) {
-	CATCH_AND_RETURN( *out = TSAV(Version, f)->get(); );
+fdb_error_t fdb_future_get_int64( FDBFuture* f, int64_t* out_value ) {
+	CATCH_AND_RETURN( *out_value = TSAV(int64_t, f)->get(); );
 }
 
 extern "C" DLLEXPORT

bindings/c/foundationdb/fdb_c.h

@@ -228,10 +228,10 @@ extern "C" {
     fdb_transaction_get_committed_version( FDBTransaction* tr,
                                            int64_t* out_version );
 
-	// This function intentionally returns an FDBFuture instead of an integer directly,
-	// so that calling this API can see the effect of previous mutations on the transaction.
-	// Specifically, mutations are applied asynchronously by the main thread. In order to
-	// see them, this call has to be serviced by the main thread too.
+    // This function intentionally returns a FDBFuture instead of an integer directly,
+    // so that calling this API can see the effect of previous mutations on the transaction.
+    // Specifically, mutations are applied asynchronously by the main thread. In order to
+    // see them, this call has to be serviced by the main thread too.
     DLLEXPORT WARN_UNUSED_RESULT FDBFuture*
     fdb_transaction_get_approximate_size(FDBTransaction* tr);
 

bindings/go/src/fdb/futures.go

@@ -331,7 +331,7 @@ func (f *futureInt64) Get() (int64, error) {
 	f.BlockUntilReady()
 
 	var ver C.int64_t
-	if err := C.fdb_future_get_version(f.ptr, &ver); err != 0 {
+	if err := C.fdb_future_get_int64(f.ptr, &ver); err != 0 {
 		return 0, Error{int(err)}
 	}
 

bindings/java/fdbJNI.cpp

@@ -260,6 +260,23 @@ JNIEXPORT jlong JNICALL Java_com_apple_foundationdb_FutureVersion_FutureVersion_
 	return (jlong)version;
 }
 
+JNIEXPORT jlong JNICALL Java_com_apple_foundationdb_FutureInt64_FutureInt64_1get(JNIEnv *jenv, jobject, jlong future) {
+	if (!future) {
+		throwParamNotNull(jenv);
+		return 0;
+	}
+	FDBFuture *f = (FDBFuture *)future;
+
+	int64_t value = 0;
+	fdb_error_t err = fdb_future_get_int64(f, &value);
+	if (err) {
+		safeThrow(jenv, getThrowable(jenv, err));
+		return 0;
+	}
+
+	return (jlong)value;
+}
+
 JNIEXPORT jobject JNICALL Java_com_apple_foundationdb_FutureStrings_FutureStrings_1get(JNIEnv *jenv, jobject, jlong future) {
 	if( !future ) {
 		throwParamNotNull(jenv);

bindings/java/src/main/com/apple/foundationdb/FDBTransaction.java

@@ -518,7 +518,7 @@ class FDBTransaction extends NativeObjectWrapper implements Transaction, OptionC
 	public CompletableFuture<Long> getApproximateSize() {
 		pointerReadLock.lock();
 		try {
-			return new FutureVersion(Transaction_getApproximateSize(getPtr()), executor);
+			return new FutureInt64(Transaction_getApproximateSize(getPtr()), executor);
 		} finally {
 			pointerReadLock.unlock();
 		}

bindings/java/src/main/com/apple/foundationdb/FutureInt64.java

@@ -0,0 +1,37 @@
+/*
+ * FutureInt64.java
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2019 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.apple.foundationdb;
+
+import java.util.concurrent.Executor;
+
+class FutureInt64 extends NativeFuture<Long> {
+	FutureInt64(long cPtr, Executor executor) {
+		super(cPtr);
+		registerMarshalCallback(executor);
+	}
+
+	@Override
+	protected Long getIfDone_internal(long cPtr) throws FDBException {
+		return FutureInt64_get(cPtr);
+	}
+
+	private native long FutureInt64_get(long cPtr) throws FDBException;
+}

bindings/python/fdb/impl.py

@@ -702,9 +702,9 @@ class FutureVersion(Future):
 class FutureInt64(Future):
     def wait(self):
         self.block_until_ready()
-        size = ctypes.c_int64()
-        self.capi.fdb_future_get_version(self.fpointer, ctypes.byref(size))
-        return size.value
+        value = ctypes.c_int64()
+        self.capi.fdb_future_get_int64(self.fpointer, ctypes.byref(value))
+        return value.value
 
 
 class FutureKeyValueArray(Future):
@@ -1375,6 +1375,10 @@ def init_c_api():
     _capi.fdb_future_get_version.restype = ctypes.c_int
     _capi.fdb_future_get_version.errcheck = check_error_code
 
+    _capi.fdb_future_get_int64.argtypes = [ctypes.c_void_p, ctypes.POINTER(ctypes.c_int64)]
+    _capi.fdb_future_get_int64.restype = ctypes.c_int
+    _capi.fdb_future_get_int64.errcheck = check_error_code
+
     _capi.fdb_future_get_key.argtypes = [ctypes.c_void_p, ctypes.POINTER(ctypes.POINTER(ctypes.c_byte)),
                                          ctypes.POINTER(ctypes.c_int)]
     _capi.fdb_future_get_key.restype = ctypes.c_int

bindings/ruby/lib/fdbimpl.rb

@@ -86,6 +86,7 @@ module FDB
 
       attach_function :fdb_future_get_error, [ :pointer ], :fdb_error
       attach_function :fdb_future_get_version, [ :pointer, :pointer ], :fdb_error
+      attach_function :fdb_future_get_int64, [ :pointer, :pointer ], :fdb_error
       attach_function :fdb_future_get_key, [ :pointer, :pointer, :pointer ], :fdb_error
       attach_function :fdb_future_get_value, [ :pointer, :pointer, :pointer, :pointer ], :fdb_error
       attach_function :fdb_future_get_keyvalue_array, [ :pointer, :pointer, :pointer, :pointer ], :fdb_error
@@ -453,6 +454,15 @@ module FDB
     private :getter
   end
 
+  class Int64Future < LazyFuture
+    def getter
+      val = FFI::MemoryPointer.new :int64
+      FDBC.check_error FDBC.fdb_future_get_int64(@fpointer, val)
+      @value = val.read_long_long
+    end
+    private :getter
+  end
+
   class FutureKeyValueArray < Future
     def wait
       block_until_ready
@@ -906,7 +916,7 @@ module FDB
     end
 
     def get_approximate_size
-      Version.new(FDBC.fdb_transaction_get_approximate_size @tpointer)
+      Int64Future.new(FDBC.fdb_transaction_get_approximate_size @tpointer)
     end
 
     def get_versionstamp

documentation/sphinx/source/api-c.rst

@@ -724,9 +724,11 @@ Applications must provide error handling and an appropriate retry loop around th
 
    Most applications will not call this function.
 
-.. function:: FDBFuture* fdb_transaction_get_approximate_size(FDBTransaction* tr)
+.. function:: FDBFuture* fdb_transaction_get_approximate_size(FDBTransaction* transaction)
 
-   Retrieves the approximate transaction size so far in the returned future, which is the summation of the estimated size of mutations, read conflict ranges, and write conflict ranges. The size can be obtained by calling :func:`fdb_future_get_int64()` with the returned :type:`FDBFuture*` object. This can be called multiple times before transaction is committed.
+  |future-return0| the approximate transaction size so far in the returned future, which is the summation of the estimated size of mutations, read conflict ranges, and write conflict ranges. |future-return1| call :func:`fdb_future_get_int64()` to extract the size, |future-return2|
+
+  This can be called multiple times before the transaction is committed.
 
 .. function:: FDBFuture* fdb_transaction_get_versionstamp(FDBTransaction* transaction)
 

fdbclient/FDBTypes.h

@@ -235,7 +235,7 @@ struct KeyRangeRef {
 		return *this;
 	}
 
-	int expectedSize() const { return begin.expectedSize() + end.expectedSize() + sizeof(KeyRangeRef); }
+	int expectedSize() const { return begin.expectedSize() + end.expectedSize(); }
 
 	template <class Ar>
 	force_inline void serialize(Ar& ar) {

fdbclient/MultiVersionTransaction.h

@@ -96,7 +96,7 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
 	//Future
 	fdb_error_t (*futureGetDatabase)(FDBFuture *f, FDBDatabase **outDb);
 	fdb_error_t (*futureGetVersion)(FDBFuture *f, int64_t *outVersion);
-	fdb_error_t (*futureGetInt64)(FDBFuture *f, int64_t *outVersion);
+	fdb_error_t (*futureGetInt64)(FDBFuture *f, int64_t *outValue);
 	fdb_error_t (*futureGetError)(FDBFuture *f);
 	fdb_error_t (*futureGetKey)(FDBFuture *f, uint8_t const **outKey, int *outKeyLength);
 	fdb_error_t (*futureGetValue)(FDBFuture *f, fdb_bool_t *outPresent, uint8_t const **outValue, int *outValueLength);