Psf functions in session #1
Conversation
- Use PSF*Exception (derived from FasterException) - Do argument validation: non-null; unique PSF names; ensure PSF belongs to the FasterKV, etc. - Minor optimization for tombstoned records that end PSF chains - Fix PSFRegistrationSettings initialization and validation - Fix up serialNum
- Remove Unsafe(Resume|Suspend)Thread in session.QueryPSF calls (it's done at a fine-grained level by session.PsfRead*) - Handle PENDING or RETRY of reads in QueryPSF - Add more TODO<tag>s
- Add binned category PSF to sample - Fix nullable return for Func<> PSF registrations - Add PSF_TRACE
- Fix RMW calls to PSF insert on NOTFOUND - Add an option to execute Before PSFs immediately (within Upsert/RMW rather than after it); this is needed for Object values where the update of the underlying value loses the original value - Rehash the TPSFKey hashcode with PSF ordinal - Implement RecordIterator and boolean QueryPSF forms with up to 3 separate TPSFKey specifications - Add PackageId and PackageOutputPath to csproj for local NuGet generation FasterPSFSample: - Add RMW to FasterPSFSample initial inserts; make Input generic on TValue - Add boolean QueryPSF examples and verification
…d-type> to IPSF - Remove FasterPSFSample launchsettings.json
- Make PSFRegistrationSettings required and move to the first parameter, to allow adding subsequent PSF specs as "params" args (which eliminated one RegisterPSF overload) - Add initial PSF Checkpoint/Restore (more work needed to ensure consistency) - Create directory if needed for LocalStorageDevice - Remove no-longer-needed saving of FasterKV ctor args for PSF
- Convert secondary FKV Key type from CompositeKey to TPSFKey - Remove IKeyAccessor as we can now use KeyAccessor<TPSFKey> directly - Also cleans up managing the single query key vs composite key handling - Add PsfQueryKeyContainer - Limit # of PSFs per group to 255
…it easier in future to omit IsNull keys from the composite key
| var isEqualKey = ctx.request_key is null | ||
| || (this.PsfKeyAccessor is null | ||
| ? comparer.Equals(ref ctx.request_key.Get(), ref GetContextRecordKey(ref ctx)) | ||
| : this.PsfKeyAccessor.EqualsAtRecordAddress(ref KeyPointer<Key>.CastFromKeyRef(ref ctx.request_key.Get()), (long)record)); |
There was a problem hiding this comment.
This (this.PsfKeyAccessor.EqualsAtRecordAddress) is for the secondary FKV, correct? We can just use this code as a custom comparer.
| if (comparer.Equals(ref ctx.request_key.Get(), ref GetContextRecordKey(ref ctx))) | ||
| // If the keys are same, then I/O is complete. For PSFs, we may be querying on an address instead of a key; | ||
| // in this case, ctx.request_key is null for the primary FKV but we know the address is the one we want. | ||
| var isEqualKey = ctx.request_key is null |
There was a problem hiding this comment.
This can use the new address-based variant of Read, instead of key-based variant of Read. Then no changes are needed.
| @@ -21,7 +21,7 @@ namespace FASTER.core | |||
| /// <typeparam name="Output"></typeparam> | |||
| /// <typeparam name="Context"></typeparam> | |||
| /// <typeparam name="Functions"></typeparam> | |||
| public sealed class ClientSession<Key, Value, Input, Output, Context, Functions> : IClientSession, IDisposable | |||
| public sealed partial class ClientSession<Key, Value, Input, Output, Context, Functions> : IClientSession, IDisposable | |||
There was a problem hiding this comment.
In FASTER.PSF.dll, we create an extension method on FASTER called NewSessionWithPSF that returns a PSFClientSession. This has a private field for ClientSession, and wraps the calls to Upsert etc. with PSF-specific crumbs. This takes care of the changes to ClientSession.cs
| @@ -257,6 +257,106 @@ public interface IFasterKV<Key, Value> : IDisposable | |||
|
|
|||
| #endregion | |||
|
|
|||
| #region PSF Registration | |||
There was a problem hiding this comment.
Move to FASTER.PSF.dll so we do not pollute IFasterKV
| @@ -160,7 +160,8 @@ public Status Read(ref Key key, ref Input input, ref Output output, Context cont | |||
| [Obsolete("Use NewSession() and invoke Upsert() on the session.")] | |||
| public Status Upsert(ref Key key, ref Value value, Context context, long serialNo) | |||
| { | |||
| return _fasterKV.ContextUpsert(ref key, ref value, context, FasterSession, serialNo, _threadCtx.Value); | |||
| PSFUpdateArgs<Key, Value> psfUpdateArgs = default; | |||
There was a problem hiding this comment.
Remove - legacy does not need to support PSF. Also ContextUpsert etc. will not have extra argument.
| @@ -74,6 +74,9 @@ internal static class Constants | |||
| public const long kInvalidAddress = 0; | |||
| public const long kTempInvalidAddress = 1; | |||
| public const int kFirstValidAddress = 64; | |||
|
|
|||
| else | ||
| { | ||
| status = HandleOperationStatus(opCtx, currentCtx, pendingContext, fasterSession, internalStatus); | ||
| switch (pendingContext.type) |
There was a problem hiding this comment.
Can't this move to the caller - after vanilla Upsert succeeds?
| @@ -402,8 +430,7 @@ public partial class FasterKV<Key, Value> : FasterBase, IFasterKV<Key, Value> | |||
| { | |||
| (Status, Output) s = default; | |||
|
|
|||
| ref Key key = ref pendingContext.key.Get(); | |||
|
|
|||
| // PSFs may read by address rather than key, and for the Primary FKV will not have pendingContext.key; this call will fill it in. | |||
There was a problem hiding this comment.
"Read by address" should be factored out instead of reusing "Read by key"
| status = OperationStatus.RETRY_NOW; | ||
| goto LatchRelease; | ||
| } | ||
|
|
||
| if (this.PSFManager.HasPSFs) |
There was a problem hiding this comment.
Move this to PSF.dll - make sure that these code fragments are isolated from rest of code so that future merges are easy.
There was a problem hiding this comment.
Can you make these code chunks a call to a method (in a PSF-specific directory), so that the main Internal* Logic are largely similar, making future merges from master easier? Instead of inlining all the stuff inside here.
For Badrish review