Merge pull request #2060 from atn34/xff-xff-get-addresses-for-key

Add database/transaction option to control includePort behavior in get_addresses_for_key

bindings/flow/tester/Tester.actor.cpp

@@ -1583,6 +1583,7 @@ struct UnitTestsFunc : InstructionFunc {
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_RETRY_LIMIT, Optional<StringRef>(StringRef((const uint8_t*)&retryLimit, 8)));
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_RETRY_LIMIT, Optional<StringRef>(StringRef((const uint8_t*)&noRetryLimit, 8)));
 		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_CAUSAL_READ_RISKY);
+		data->db->setDatabaseOption(FDBDatabaseOption::FDB_DB_OPTION_TRANSACTION_INCLUDE_PORT_IN_ADDRESS);
 
 		state Reference<Transaction> tr = data->db->createTransaction();
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_PRIORITY_SYSTEM_IMMEDIATE);
@@ -1601,6 +1602,7 @@ struct UnitTestsFunc : InstructionFunc {
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_TRANSACTION_LOGGING_ENABLE, Optional<StringRef>(LiteralStringRef("my_transaction")));
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_READ_LOCK_AWARE);
 		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_LOCK_AWARE);
+		tr->setOption(FDBTransactionOption::FDB_TR_OPTION_INCLUDE_PORT_IN_ADDRESS);
 
 		Optional<FDBStandalone<ValueRef> > _ = wait(tr->get(LiteralStringRef("\xff")));
 		tr->cancel();

bindings/go/src/_stacktester/stacktester.go

@@ -805,6 +805,7 @@ func (sm *StackMachine) processInst(idx int, inst tuple.Tuple) {
 		db.Options().SetTransactionRetryLimit(10)
 		db.Options().SetTransactionRetryLimit(-1)
 		db.Options().SetTransactionCausalReadRisky()
+		db.Options().SetTransactionIncludePortInAddress()
 
 		if !fdb.IsAPIVersionSelected() {
 			log.Fatal("API version should be selected")
@@ -850,6 +851,7 @@ func (sm *StackMachine) processInst(idx int, inst tuple.Tuple) {
 			tr.Options().SetLogTransaction()
 			tr.Options().SetReadLockAware()
 			tr.Options().SetLockAware()
+			tr.Options().SetIncludePortInAddress()
 
 			return tr.Get(fdb.Key("\xff")).MustGet(), nil
 		})

bindings/go/src/fdb/generated.go

@@ -46,13 +46,6 @@ func (o NetworkOptions) SetLocalAddress(param string) error {
 	return o.setOpt(10, []byte(param))
 }
 
-// enable the object serializer for network communication
-//
-// Parameter: 0 is false, every other value is true
-func (o NetworkOptions) SetUseObjectSerializer(param int64) error {
-	return o.setOpt(11, int64ToBytes(param))
-}
-
 // Deprecated
 //
 // Parameter: path to cluster file
@@ -330,6 +323,11 @@ func (o DatabaseOptions) SetTransactionCausalReadRisky() error {
 	return o.setOpt(504, nil)
 }
 
+// Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated.
+func (o DatabaseOptions) SetTransactionIncludePortInAddress() error {
+	return o.setOpt(505, nil)
+}
+
 // The transaction, if not self-conflicting, may be committed a second time after commit succeeds, in the event of a fault
 func (o TransactionOptions) SetCausalWriteRisky() error {
 	return o.setOpt(10, nil)
@@ -345,6 +343,11 @@ func (o TransactionOptions) SetCausalReadDisable() error {
 	return o.setOpt(21, nil)
 }
 
+// Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated.
+func (o TransactionOptions) SetIncludePortInAddress() error {
+	return o.setOpt(23, nil)
+}
+
 // The next write performed on this transaction will not generate a write conflict range. As a result, other transactions which read the key(s) being modified by the next write will not conflict with this transaction. Care needs to be taken when using this option on a transaction that is shared between multiple threads. When setting this option, write conflict ranges will be disabled on the next write operation, regardless of what thread it is on.
 func (o TransactionOptions) SetNextWriteNoWriteConflictRange() error {
 	return o.setOpt(30, nil)
@@ -508,7 +511,7 @@ const (
 	// Infrequently used. The client has passed a specific row limit and wants
 	// that many rows delivered in a single batch. Because of iterator operation
 	// in client drivers make request batches transparent to the user, consider
-	// ``WANT_ALL`` StreamingMode instead. A row limit must be specified if this
+	// “WANT_ALL“ StreamingMode instead. A row limit must be specified if this
 	// mode is used.
 	StreamingModeExact StreamingMode = 1
 
@@ -625,15 +628,15 @@ type ErrorPredicate int
 
 const (
 
-	// Returns ``true`` if the error indicates the operations in the
+	// Returns “true“ if the error indicates the operations in the transactions
-	// transactions should be retried because of transient error.
+	// should be retried because of transient error.
 	ErrorPredicateRetryable ErrorPredicate = 50000
 
-	// Returns ``true`` if the error indicates the transaction may have
+	// Returns “true“ if the error indicates the transaction may have succeeded,
-	// succeeded, though not in a way the system can verify.
+	// though not in a way the system can verify.
 	ErrorPredicateMaybeCommitted ErrorPredicate = 50001
 
-	// Returns ``true`` if the error indicates the transaction has not
+	// Returns “true“ if the error indicates the transaction has not committed,
-	// committed, though in a way that can be retried.
+	// though in a way that can be retried.
 	ErrorPredicateRetryableNotCommitted ErrorPredicate = 50002
 )

bindings/java/src/test/com/apple/foundationdb/test/AsyncStackTester.java

@@ -495,6 +495,7 @@ public class AsyncStackTester {
 				db.options().setTransactionRetryLimit(10);
 				db.options().setTransactionRetryLimit(-1);
 				db.options().setTransactionCausalReadRisky();
+				db.options().setTransactionIncludePortInAddress();
 
 				tr.options().setPrioritySystemImmediate();
 				tr.options().setPriorityBatch();
@@ -512,6 +513,7 @@ public class AsyncStackTester {
 				tr.options().setLogTransaction();
 				tr.options().setReadLockAware();
 				tr.options().setLockAware();
+				tr.options().setIncludePortInAddress();
 
 				if(!(new FDBException("Fake", 1020)).isRetryable() ||
 						(new FDBException("Fake", 10)).isRetryable())

bindings/java/src/test/com/apple/foundationdb/test/StackTester.java

@@ -447,6 +447,7 @@ public class StackTester {
 						db.options().setTransactionRetryLimit(10);
 						db.options().setTransactionRetryLimit(-1);
 						db.options().setTransactionCausalReadRisky();
+						db.options().setTransactionIncludePortInAddress();
 
 						tr.options().setPrioritySystemImmediate();
 						tr.options().setPriorityBatch();
@@ -464,6 +465,7 @@ public class StackTester {
 						tr.options().setLogTransaction();
 						tr.options().setReadLockAware();
 						tr.options().setLockAware();
+						tr.options().setIncludePortInAddress();
 
 						if(!(new FDBException("Fake", 1020)).isRetryable() ||
 								(new FDBException("Fake", 10)).isRetryable())

bindings/python/tests/tester.py

@@ -143,6 +143,7 @@ def test_db_options(db):
     db.options.set_transaction_retry_limit(10)
     db.options.set_transaction_retry_limit(-1)
     db.options.set_transaction_causal_read_risky()
+    db.options.set_transaction_include_port_in_address()
 
 
 @fdb.transactional
@@ -163,6 +164,7 @@ def test_options(tr):
     tr.options.set_log_transaction()
     tr.options.set_read_lock_aware()
     tr.options.set_lock_aware()
+    tr.options.set_include_port_in_address()
 
     tr.get(b'\xff').wait()
 

bindings/ruby/tests/tester.rb

@@ -469,6 +469,7 @@ class Tester
             @db.options.set_transaction_retry_limit(10)
             @db.options.set_transaction_retry_limit(-1)
             @db.options.set_transaction_causal_read_risky()
+            @db.options.set_transaction_include_port_in_address()
 
             @db.transact do |tr|
               tr.options.set_priority_system_immediate
@@ -487,6 +488,7 @@ class Tester
               tr.options.set_log_transaction()
               tr.options.set_read_lock_aware()
               tr.options.set_lock_aware()
+              tr.options.set_include_port_in_address()
 
               tr.get("\xff").to_s
             end

documentation/sphinx/source/api-c.rst

@@ -51,6 +51,8 @@
 .. |timeout-database-option| replace:: FIXME
 .. |causal-read-risky-transaction-option| replace:: FIXME
 .. |causal-read-risky-database-option| replace:: FIXME
+.. |include-port-in-address-database-option| replace:: FIXME
+.. |include-port-in-address-transaction-option| replace:: FIXME
 .. |transaction-logging-max-field-length-database-option| replace:: FIXME
 .. |transaction-logging-max-field-length-transaction-option| replace:: FIXME
 

documentation/sphinx/source/api-common.rst.inc

@@ -322,6 +322,10 @@
 
     Transactions do not require the strict causal consistency guarantee that FoundationDB provides by default.  The read version will be committed, and usually will be the latest committed, but might not be the latest committed in the event of a simultaneous fault and misbehaving clock. Enabling this option is equivalent to calling |causal-read-risky-transaction-option| on each transaction created by this database.
 
+.. |option-db-include-port-in-address-blurb| replace::
+
+    Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated. Enabling this option is equivalent to calling |include-port-in-address-transaction-option| on each transaction created by this database.
+
 .. |option-db-snapshot-ryw-enable-blurb| replace::
 
     If this option has been set an equal or more times with this database than the disable option, snapshot reads *will* see the effects of prior writes in the same transaction. Enabling this option is equivalent to calling |snapshot-ryw-enable-transaction-option| on each transaction created by this database.
@@ -361,6 +365,10 @@
 
     This transaction does not require the strict causal consistency guarantee that FoundationDB provides by default.  The read version will be committed, and usually will be the latest committed, but might not be the latest committed in the event of a simultaneous fault and misbehaving clock. One can set this for all transactions by calling |causal-read-risky-database-option|.
 
+.. |option-include-port-in-address-blurb| replace::
+
+    Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated. One can set this for all transactions by calling |include-port-in-address-database-option|.
+
 .. |option-causal-write-risky-blurb| replace::
 
     The application either knows that this transaction will be self-conflicting (at least one read overlaps at least one set or clear), or is willing to accept a small risk that the transaction could be committed a second time after its commit apparently succeeds.  This option provides a small performance benefit.

documentation/sphinx/source/api-python.rst

@@ -26,6 +26,7 @@
 .. |max-retry-delay-database-option| replace:: :func:`Database.options.set_transaction_max_retry_delay`
 .. |transaction-size-limit-database-option| replace:: :func:`Database.options.set_transaction_size_limit`
 .. |causal-read-risky-database-option| replace:: :func:`Database.options.set_transaction_causal_read_risky`
+.. |include-port-in-address-database-option| replace:: :func:`Database.options.set_transaction_include_port_in_address`
 .. |transaction-logging-max-field-length-database-option| replace:: :func:`Database.options.set_transaction_logging_max_field_length`
 .. |snapshot-ryw-enable-database-option| replace:: :func:`Database.options.set_snapshot_ryw_enable`
 .. |snapshot-ryw-disable-database-option| replace:: :func:`Database.options.set_snapshot_ryw_disable`
@@ -38,6 +39,7 @@
 .. |snapshot-ryw-enable-transaction-option| replace:: :func:`Transaction.options.set_snapshot_ryw_enable`
 .. |snapshot-ryw-disable-transaction-option| replace:: :func:`Transaction.options.set_snapshot_ryw_disable`
 .. |causal-read-risky-transaction-option| replace:: :func:`Transaction.options.set_causal_read_risky`
+.. |include-port-in-address-transaction-option| replace:: :func:`Transaction.options.set_include_port_in_address`
 .. |transaction-logging-max-field-length-transaction-option| replace:: :func:`Transaction.options.set_transaction_logging_max_field_length`
 .. |lazy-iterator-object| replace:: generator
 .. |key-meth| replace:: :meth:`Subspace.key`
@@ -392,6 +394,10 @@ Database options
 
     |option-db-causal-read-risky-blurb|
 
+.. method:: Database.options.set_transaction_include_port_in_address()
+
+    |option-db-include-port-in-address-blurb|
+    
 .. method:: Database.options.set_transaction_logging_max_field_length(size_limit)
 
     |option-db-tr-transaction-logging-max-field-length-blurb|
@@ -809,6 +815,10 @@ Transaction options
 
     |option-causal-read-risky-blurb|
 
+.. method:: Transaction.options.set_include_port_in_address
+
+    |option-include-port-in-address-blurb|
+
 .. method:: Transaction.options.set_causal_write_risky
 
     |option-causal-write-risky-blurb|

documentation/sphinx/source/api-ruby.rst

@@ -24,6 +24,7 @@
 .. |max-retry-delay-database-option| replace:: :meth:`Database.options.set_transaction_max_retry_delay`
 .. |transaction-size-limit-database-option| replace:: :func:`Database.options.set_transaction_size_limit`
 .. |causal-read-risky-database-option| replace:: :meth:`Database.options.set_transaction_causal_read_risky`
+.. |include-port-in-address-database-option| replace:: :meth:`Database.options.set_transaction_include_port_in_address`
 .. |snapshot-ryw-enable-database-option| replace:: :meth:`Database.options.set_snapshot_ryw_enable`
 .. |snapshot-ryw-disable-database-option| replace:: :meth:`Database.options.set_snapshot_ryw_disable`
 .. |transaction-logging-max-field-length-database-option| replace:: :meth:`Database.options.set_transaction_logging_max_field_length`
@@ -36,6 +37,7 @@
 .. |snapshot-ryw-enable-transaction-option| replace:: :meth:`Transaction.options.set_snapshot_ryw_enable`
 .. |snapshot-ryw-disable-transaction-option| replace:: :meth:`Transaction.options.set_snapshot_ryw_disable`
 .. |causal-read-risky-transaction-option| replace:: :meth:`Transaction.options.set_causal_read_risky`
+.. |include-port-in-address-transaction-option| replace:: :meth:`Transaction.options.set_include_port_in_address`
 .. |transaction-logging-max-field-length-transaction-option| replace:: :meth:`Transaction.options.set_transaction_logging_max_field_length`
 .. |lazy-iterator-object| replace:: :class:`Enumerator`
 .. |key-meth| replace:: :meth:`Subspace.key`
@@ -388,6 +390,10 @@ Database options
 
     |option-db-causal-read-risky-blurb|
 
+.. method:: Database.options.set_transaction_include_port_in_address() -> nil
+
+    |option-db-include-port-in-address-blurb|
+
 .. method:: Database.options.set_transaction_logging_max_field_length(size_limit) -> nil
 
     |option-db-tr-transaction-logging-max-field-length-blurb|
@@ -755,6 +761,10 @@ Transaction options
 
     |option-causal-read-risky-blurb|
 
+.. method:: Transaction.options.set_include_port_in_address() -> nil
+
+    |option-include-port-in-address-blurb|
+
 .. method:: Transaction.options.set_causal_write_risky() -> nil
 
     |option-causal-write-risky-blurb|

documentation/sphinx/source/data-modeling.rst

@@ -53,6 +53,8 @@
 .. |timeout-database-option| replace:: FIXME
 .. |causal-read-risky-database-option| replace:: FIXME
 .. |causal-read-risky-transaction-option| replace:: FIXME
+.. |include-port-in-address-database-option| replace:: FIXME
+.. |include-port-in-address-transaction-option| replace:: FIXME
 .. |transaction-logging-max-field-length-transaction-option| replace:: FIXME
 .. |transaction-logging-max-field-length-database-option| replace:: FIXME
 

documentation/sphinx/source/developer-guide.rst

@@ -53,6 +53,8 @@
 .. |timeout-database-option| replace:: FIXME
 .. |causal-read-risky-database-option| replace:: FIXME
 .. |causal-read-risky-transaction-option| replace:: FIXME
+.. |include-port-in-address-database-option| replace:: FIXME
+.. |include-port-in-address-transaction-option| replace:: FIXME
 .. |transaction-logging-max-field-length-transaction-option| replace:: FIXME
 .. |transaction-logging-max-field-length-database-option| replace:: FIXME
 

documentation/sphinx/source/release-notes.rst

@@ -73,6 +73,7 @@ Bindings
 * Go: The Go bindings now require Go version 1.11 or later.
 * Go: Finalizers could run too early leading to undefined behavior. `(PR #1451) <https://github.com/apple/foundationdb/pull/1451>`_.
 * Added a transaction option to control the field length of keys and values in debug transaction logging in order to avoid truncation. `(PR #1844) <https://github.com/apple/foundationdb/pull/1844>`_.
+* Added a transaction option to control the whether ``get_addresses_for_key`` includes a port in the address. This will be deprecated in api version 700, and addresses will include ports by default. `(PR #2060) <https://github.com/apple/foundationdb/pull/2060>`_.
 
 Other Changes
 -------------

fdbclient/NativeAPI.actor.cpp

@@ -2137,7 +2137,9 @@ Future< Void > Transaction::watch( Reference<Watch> watch ) {
 	return ::watch(watch, cx, this);
 }
 
-ACTOR Future< Standalone< VectorRef< const char*>>> getAddressesForKeyActor( Key key, Future<Version> ver, Database cx, TransactionInfo info ) {
+ACTOR Future<Standalone<VectorRef<const char*>>> getAddressesForKeyActor(Key key, Future<Version> ver, Database cx,
+                                                                         TransactionInfo info,
+                                                                         TransactionOptions options) {
 	state vector<StorageServerInterface> ssi;
 
 	// If key >= allKeys.end, then getRange will return a kv-pair with an empty value. This will result in our serverInterfaces vector being empty, which will cause us to return an empty addresses list.
@@ -2158,7 +2160,7 @@ ACTOR Future< Standalone< VectorRef< const char*>>> getAddressesForKeyActor( Key
 
 	Standalone<VectorRef<const char*>> addresses;
 	for (auto i : ssi) {
-		std::string ipString = i.address().ip.toString();
+		std::string ipString = options.includePort ? i.address().toString() : i.address().ip.toString();
 		char* c_string = new (addresses.arena()) char[ipString.length()+1];
 		strcpy(c_string, ipString.c_str());
 		addresses.push_back(addresses.arena(), c_string);
@@ -2170,7 +2172,7 @@ Future< Standalone< VectorRef< const char*>>> Transaction::getAddressesForKey( c
 	++cx->transactionLogicalReads;
 	auto ver = getReadVersion();
 
-	return getAddressesForKeyActor(key, ver, cx, info);
+	return getAddressesForKeyActor(key, ver, cx, info, options);
 }
 
 ACTOR Future< Key > getKeyAndConflictRange(
@@ -2968,6 +2970,11 @@ void Transaction::setOption( FDBTransactionOptions::Option option, Optional<Stri
 			info.useProvisionalProxies = true;
 			break;
 
+		case FDBTransactionOptions::INCLUDE_PORT_IN_ADDRESS:
+			validateOptionValue(value, false);
+			options.includePort = true;
+			break;
+
 		default:
 			break;
 	}

fdbclient/NativeAPI.actor.h

@@ -131,6 +131,7 @@ struct TransactionOptions {
 	bool lockAware : 1;
 	bool readOnly : 1;
 	bool firstInBatch : 1;
+	bool includePort : 1;
 
 	TransactionOptions(Database const& cx);
 	TransactionOptions();

fdbclient/vexillographer/fdb.options

@@ -171,6 +171,9 @@ description is not currently required but encouraged.
     <Option name="transaction_causal_read_risky" code="504"
             description="The read version will be committed, and usually will be the latest committed, but might not be the latest committed in the event of a simultaneous fault and misbehaving clock."
             defaultFor="20"/>
+    <Option name="transaction_include_port_in_address" code="505"
+            description="Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated."
+            defaultFor="23"/>
   </Scope>
   
   <Scope name="TransactionOption">
@@ -179,6 +182,8 @@ description is not currently required but encouraged.
     <Option name="causal_read_risky" code="20"
             description="The read version will be committed, and usually will be the latest committed, but might not be the latest committed in the event of a simultaneous fault and misbehaving clock."/>
     <Option name="causal_read_disable" code="21" />
+    <Option name="include_port_in_address" code="23"
+            description="Addresses returned by get_addresses_for_key include the port when enabled. This will be enabled by default in api version 700, and this option will be deprecated." />
     <Option name="next_write_no_write_conflict_range" code="30"
             description="The next write performed on this transaction will not generate a write conflict range. As a result, other transactions which read the key(s) being modified by the next write will not conflict with this transaction. Care needs to be taken when using this option on a transaction that is shared between multiple threads. When setting this option, write conflict ranges will be disabled on the next write operation, regardless of what thread it is on." />
     <Option name="commit_on_first_proxy" code="40"