Skip to content

Commit

Permalink
Promote value-chekcs as a first class operation supported by the tran…
Browse files Browse the repository at this point in the history
…saction object.

- tr.CheckValueAsync(key, expected) will read the actual value and compare it to expected value. Will return a pair with the FdbValueCheckResult and actual value.
- This command is the logical equivalent of "(await GetAsync(key)) == expected" but is optimized to reduce memory allocation in the case where the check passes.
- Rewrote deferred value-checks to use this command (cleans up the logs)
- Renamed ValueCheckFailedInPreviousAttempt(..) into TestValueCheckFromPreviousAttempt(..) since we now return an enum
- Renamed ListValueChecksFromPreviousAttempt(..) into GetValueChecksFromPreviousAttempt(..)
- Removed the singleton flag HasAtLeastOneFailedValueCheck because it is the equivalent of a giant lock, and each layer should use its own tag anyway.
- Added a few more annotation in the transaction logs to help troubleshoot value checks.
  • Loading branch information
KrzysFR committed May 26, 2020
1 parent 0a11832 commit 5ed5cd1
Show file tree
Hide file tree
Showing 11 changed files with 316 additions and 79 deletions.
8 changes: 8 additions & 0 deletions FoundationDB.Client/Core/IFdbTransactionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ public interface IFdbTransactionHandler : IDisposable
/// <returns></returns>
Task<FdbRangeChunk> GetRangeAsync(KeySelector beginInclusive, KeySelector endExclusive, int limit, bool reverse, int targetBytes, FdbStreamingMode mode, FdbReadMode read, int iteration, bool snapshot, CancellationToken ct);

/// <summary>Check the the value of a key in the database snapshot is equal to the expected value.</summary>
/// <param name="key">Key to check</param>
/// <param name="expected">Expected value of the key</param>
/// <param name="snapshot">Set to true for snapshot reads</param>
/// <param name="ct">Token used to cancel the operation from the outside</param>
/// <returns>Task that will return a pair of <see cref="FdbValueCheckResult"/> and the actual value of the key in the database.</returns>
Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(ReadOnlySpan<byte> key, Slice expected, bool snapshot, CancellationToken ct);

/// <summary>Returns a list of public network addresses as strings, one for each of the storage servers responsible for storing <paramref name="key"/> and its associated value</summary>
/// <param name="key">Name of the key whose location is to be queried.</param>
/// <param name="ct">Token used to cancel the operation from the outside</param>
Expand Down
81 changes: 38 additions & 43 deletions FoundationDB.Client/FdbOperationContext.cs

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions FoundationDB.Client/FdbTransaction.Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,20 @@ public Task<string[]> GetAddressesForKeyAsync(ReadOnlySpan<byte> key)
return m_parent.PerformGetAddressesForKeyOperation(key);
}

/// <inheritdoc />
public Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(ReadOnlySpan<byte> key, Slice expected)
{
EnsureCanRead();

FdbKey.EnsureKeyIsValid(key);

#if DEBUG
if (Logging.On && Logging.IsVerbose) Logging.Verbose(this, "ValueCheckAsync", $"Checking the value for '{key.ToString()}'");
#endif

return m_parent.PerformValueCheckOperation(key, expected, snapshot: true);
}

void IFdbReadOnlyTransaction.Cancel()
{
throw new NotSupportedException("You cannot cancel the Snapshot view of a transaction.");
Expand Down
30 changes: 30 additions & 0 deletions FoundationDB.Client/FdbTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,36 @@ private Task<Slice> PerformGetOperation(ReadOnlySpan<byte> key, bool snapshot)
}
}

/// <inheritdoc />
public Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(ReadOnlySpan<byte> key, Slice expected)
{
EnsureCanRead();

FdbKey.EnsureKeyIsValid(key);

#if DEBUG
if (Logging.On && Logging.IsVerbose) Logging.Verbose(this, "ValueCheckAsync", $"Checking the value for '{key.ToString()}'");
#endif

return PerformValueCheckOperation(key, expected, snapshot: false);
}

private Task<(FdbValueCheckResult Result, Slice Actual)> PerformValueCheckOperation(ReadOnlySpan<byte> key, Slice expected, bool snapshot)
{
if (m_log != null)
{
return m_log.ExecuteAsync(
this,
new FdbTransactionLog.CheckValueCommand(m_log.Grab(key), m_log.Grab(expected)) { Snapshot = snapshot },
(tr, cmd) => tr.m_handler.CheckValueAsync(cmd.Key.Span, cmd.Expected, cmd.Snapshot, tr.m_cancellation)
);
}
else
{
return m_handler.CheckValueAsync(key, expected, snapshot: snapshot, m_cancellation);
}
}

#endregion

#region GetValues...
Expand Down
19 changes: 19 additions & 0 deletions FoundationDB.Client/FdbTransactionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,25 @@ public static Task<string[]> GetAddressesForKeyAsync(this IFdbReadOnlyTransactio

#endregion

#region CheckValueAsync...

/// <summary>Check if the value from the database snapshot represented by the current transaction is equal to some <paramref name="expected"/> value.</summary>
/// <param name="key">Key to be looked up in the database</param>
/// <param name="expected">Expected value for this key</param>
/// <returns>Task that will return the value of the key if it is found, Slice.Nil if the key does not exist, or an exception</returns>
/// <exception cref="System.ArgumentException">If the <paramref name="key"/> is null</exception>
/// <exception cref="System.OperationCanceledException">If the cancellation token is already triggered</exception>
/// <exception cref="System.ObjectDisposedException">If the transaction has already been completed</exception>
/// <exception cref="System.InvalidOperationException">If the operation method is called from the Network Thread</exception>
/// <returns>Return the result of the check, plus the actual value of the key.</returns>
public static Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(this IFdbReadOnlyTransaction trans, Slice key, Slice expected)
{
if (key.IsNull) throw Fdb.Errors.KeyCannotBeNull();
return trans.CheckValueAsync(key.Span, expected);
}

#endregion

#region Clear...

/// <summary>
Expand Down
11 changes: 11 additions & 0 deletions FoundationDB.Client/IFdbReadOnlyTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,17 @@ Task<FdbRangeChunk> GetRangeAsync(
[Pure, LinqTunnel]
FdbRangeQuery<TResult> GetRange<TResult>(KeySelector beginInclusive, KeySelector endExclusive, Func<KeyValuePair<Slice, Slice>, TResult> selector, FdbRangeOptions? options = null);

/// <summary>Check if the value from the database snapshot represented by the current transaction is equal to some <paramref name="expected"/> value.</summary>
/// <param name="key">Key to be looked up in the database</param>
/// <param name="expected">Expected value for this key</param>
/// <returns>Task that will return the value of the key if it is found, Slice.Nil if the key does not exist, or an exception</returns>
/// <exception cref="System.ArgumentException">If the <paramref name="key"/> is null</exception>
/// <exception cref="System.OperationCanceledException">If the cancellation token is already triggered</exception>
/// <exception cref="System.ObjectDisposedException">If the transaction has already been completed</exception>
/// <exception cref="System.InvalidOperationException">If the operation method is called from the Network Thread</exception>
/// <returns>Return the result of the check, plus the actual value of the key.</returns>
Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(ReadOnlySpan<byte> key, Slice expected);

/// <summary>Returns a list of public network addresses as strings, one for each of the storage servers responsible for storing <paramref name="key"/> and its associated value</summary>
/// <param name="key">Name of the key whose location is to be queried.</param>
/// <returns>Task that will return an array of strings, or an exception</returns>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ internal async ValueTask<CacheContext> GetContext(IFdbReadOnlyTransaction trans)
var context = Volatile.Read(ref this.Context) ?? this.Layer.Cache;
if (context != null)
{
if (trans.Context.ValueCheckFailedInPreviousAttempt("DirectoryLayer") != FdbValueCheckResult.Failed)
if (trans.Context.TestValueCheckFromPreviousAttempt("DirectoryLayer") != FdbValueCheckResult.Failed)
{ // all good!
if (AnnotateTransactions) trans.Annotate($"{this.Layer} cache context #{context.ReadVersion} likely still valid (no failed value-checks at attempt #{trans.Context.Retries})");
return context;
Expand Down
30 changes: 30 additions & 0 deletions FoundationDB.Client/Native/FdbNativeTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,18 @@ public void SetReadVersion(long version)
FdbNative.TransactionSetReadVersion(m_handle, version);
}

private static bool TryPeekValueResultBytes(FutureHandle h, out ReadOnlySpan<byte> result)
{
Contract.Requires(h != null);
var err = FdbNative.FutureGetValue(h, out bool present, out result);
#if DEBUG_TRANSACTIONS
Debug.WriteLine("FdbTransaction[].TryPeekValueResultBytes() => err=" + err + ", present=" + present + ", valueLength=" + result.Count);
#endif
Fdb.DieOnError(err);

return present;
}

private static Slice GetValueResultBytes(FutureHandle h)
{
Contract.Requires(h != null);
Expand Down Expand Up @@ -328,7 +340,25 @@ public Task<Slice[]> GetKeysAsync(KeySelector[] selectors, bool snapshot, Cancel
throw;
}
return FdbFuture.CreateTaskFromHandleArray(futures, (h) => GetKeyResult(h), ct);
}

public Task<(FdbValueCheckResult Result, Slice Actual)> CheckValueAsync(ReadOnlySpan<byte> key, Slice expected, bool snapshot, CancellationToken ct)
{
return FdbFuture.CreateTaskFromHandle(
FdbNative.TransactionGet(m_handle, key, snapshot),
(h) =>
{
if (TryPeekValueResultBytes(h, out var actual))
{ // key exists
return !expected.IsNull && expected.Span.SequenceEqual(actual) ? (FdbValueCheckResult.Success, expected) : (FdbValueCheckResult.Failed, Slice.Copy(actual));
}
else
{ // key does not exist, pass only if expected is Nil
return expected.IsNull ? (FdbValueCheckResult.Success, Slice.Nil) : (FdbValueCheckResult.Failed, Slice.Nil);
}
},
ct
);
}

#endregion
Expand Down
33 changes: 33 additions & 0 deletions FoundationDB.Client/Tracing/FdbTransactionLog.Commands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,39 @@ public override string GetResult(KeyResolver resolver)

}

public sealed class CheckValueCommand : Command<(FdbValueCheckResult Result, Slice Actual)>
{
/// <summary>Selector to a key in the database</summary>
public Slice Key { get; }

/// <summary>Selector to a key in the database</summary>
public Slice Expected { get; }

public override Operation Op => Operation.CheckValue;

public CheckValueCommand(Slice key, Slice expected)
{
this.Key = key;
this.Expected = expected;
}

public override int? ArgumentBytes => this.Key.Count + this.Expected.Count;

public override int? ResultBytes => !this.Result.HasValue ? default(int?) : this.Result.Value.Actual.Count;

public override string GetArguments(KeyResolver resolver)
{
return resolver.Resolve(this.Key) + " =?= " + (this.Expected.IsNull ? "<missing>" : this.Expected.ToString("V"));
}

protected override string Dump((FdbValueCheckResult Result, Slice Actual) value)
{
return value.Actual.IsNull ? $"<missing> [{value.Result}]" : $"{value.Actual:V} [{value.Result}]";
}

}


public sealed class GetVersionStampCommand : Command<VersionStamp>
{
public override Operation Op => Operation.GetVersionStamp;
Expand Down
1 change: 1 addition & 0 deletions FoundationDB.Client/Tracing/FdbTransactionLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ public enum Operation
GetValues,
GetKeys,
GetRange,
CheckValue,
Watch,

GetReadVersion,
Expand Down
Loading

0 comments on commit 5ed5cd1

Please sign in to comment.