Caching-related enhancements

[gkc]

NB

Original bug report preserved below, but we've split off the bug into a separate ticket (completed), and this ticket is now for discussion of enhancements we can make related to caching of data, and essentially two questions:

1. should receiver atSign secondary always cache (currently leaning towards 'yes')
2. should we make it possible for receiver atSign secondary to decide to cache just the keys, not the values? (currently leaning towards 'yes')

========================================================================================== Original bug report

Describe the bug Two problems here:

1. getKeys, when specifying sharedBy, results in a new one-time-use-only connection to the cloud secondary, by creating a new RemoteSecondary object --> New issue created here (Completed)
2. But why is it connecting to the cloud secondary at all? All of the info should have synced to local secondary. Indeed, if you don't specify sharedBy, it just scans the local key store. Why is this?

Expected behaviour

• If it's really necessary to go to cloud secondary (possibly we could have a parameter for this) then it should re-use the already-created RemoteSecondary, not create a new one with all the overhead that brings
• But it seems like it should not be necessary to go to the cloud secondary. ?

[gkc]

Looking at the server-side code, when sharedBy is supplied it results in the current atSign's cloud secondary connecting to sharedBy's cloud secondary and scanning there.

@cconstab @nickelskevin FYI as to why this is necessary - it's because if alice shares something with bob, it is only sent to bob's key store if ttr is non-null and not equal to zero. Thus bob can only be sure to see everything that alice has shared with them if bob secondary scans alice secondary

@bob@ scan
  => data:["@bob:signing_privatekey@bob","cached:@bob:key_with_ttr_equal_to_minus_one.__test@alice",
"cached:@bob:key_with_ttr_equal_to_one.__test@alice","cached:@bob:shared_key@alice",
"public:publickey@bob","public:signing_publickey@bob"]

@bob@ scan:@alice
  => data:["key1.__test@alice","key_with_ttr_equal_to_minus_one.__test@alice",
"key_with_ttr_equal_to_null.__test@alice", 
"key_with_ttr_equal_to_one.__test@alice","key_with_ttr_equal_to_zero.__test@alice",
"shared_key@alice"]

[gkc]

@cconstab @nickelskevin I think we need to review this area, as the behaviour for shared data with ttr == {null | zero} could be confusing / surprising on the client side when doing scans / gets / etc.

I think we should talk about the following

• 1 The use case for prevention of caching on sharedWith's secondary
• 2a) If we re-affirm that we need the ability to prevent caching, what do we need to change end-to-end in order to adhere to the principle of least surprise
• 2b) If we decide that we don't need the ability to prevent caching, what do we need to change? (This is potentially as straightforward as treating null and 0 in the same way as we currently treat ttr == -1 - i.e. cache indefinitely)

Additionally I think we should consider caching from perspective of both sender and receiver

• Sender wants to make a statement about how long the value can be considered 'fresh' and can therefore be cached
• Receiver may wish to prevent a whole bunch of data being stored on their secondary.
• ... others

Lastly - in light of the fact that we now have reliable way (autoNotify) of delivering updates from alice to bob if alice changes some data, should we reconsider the whole ttr concept? To put it another way - if we are guaranteeing (as we are, now) that bob will always get timely updates from alice when alice creates / updates / deletes data they've shared with bob, then what is the value of ttr?

[gkc]

@gkc and @nickelskevin discussing

• ttr for ensuring fresh data is obsolete for secondaries which are both implementing notifications (and autoNotify)
• Can see situations where future implementers of atProtocol might not wish to honour notifications or at least autoNotify
• The receiver opinion matters as well as the sender. Receiver might not want to cache anything, or some things.
• In fact really, it seems like it's only the receiver's opinion that should matter

[Aside: The important operative parameter is ttl]

Some conclusions / proposals:

1. Regardless of what the sender sets for ttr, the receiver should always cache. 1.1 ttr has no obvious use in a world where we have autoNotify, but we believe we should keep it in the protocol as it may have value in other implementations of atProtocol
2. But the receiver secondary really only needs to cache the key and the metadata, not the value
3. Clients have the need to store the shared value

This is relatively easily achievable and allows full consistency end-to-end and no surprises if (proposal):

• Receiver secondary always creates an entry in the keyStore, but sets value to null and adds another flag indicating that an actual lookup will go to the sender's secondary to retrieve the value
• A 'get' on the value, in the receiver's secondary, results in a lookup on the cached entry, observation that the 'getFromSender' flag is set, and thus a lookup on the sender's secondary
• Every entry that is shared will have an entry in the receiver's keyStore - and these will be synced to interested clients
• When a client does a 'get' on the value, it observes the 'getFromSender' value, and does lookup to its cloud secondary, which in turn, as described above, will observe the getFromSender flag and do lookup on the sender's secondary
• Clients should have the option to cache the value in their local data store. This is fine, because if the value is updated by the sender, a notification will flow through and result in the receiver secondary data store being updated, and subsequently synced to the client, resetting the value to null, so the client can know that it needs to look up the value again.

Going back to why we are talking about this in the first place:

• AtClient.getKeys can now always just operate on local data store, as (subject to being in sync) the local secondary data store will always have an entry for anything that was shared by another atSign
• We should add a parameter which results in the client doing the scan on the remote secondary - this will be super useful for checking sync consistency by the way

[gkc]

Need to split this into two tickets (1) Short-term : Change LocalSecondary._scan so it doesn't create a new RemoteSecondary object (and thus a new connection) every time we do a getKeys where we are supplying sharedBy =>> Created new ticket, #759 (Complete) (2) Medium-term (this ticket): Changes discussed above as an enhancement

[gkc]

@nickelskevin we didn't discuss public keys, where I think ttr does have a role. I think the same behaviour we discussed above still apply, but we should talk through this scenario also

[gkc]

Adding something that I wrote in another conversation:

Here's the way I think all of this should work: cached should not be a part of the key name. When @bob creates data which they share with @alice, and @alice secondary decides to cache that data (for whatever reason), then I believe there is zero value in prepending 'cached:' to the key name, since the very fact that @bob-owned data is stored on @alice's secondary shows that it is cached.

[gkc]

No progress in PR48 - moving to backlog

[JeremyTubongbanua]

@gkc I made a public key on my secondary and it automatically made ttr:0 (so therefore should not be cached when looked up).

@smoothalligator's secondary with public key ttr 0

@smoothalligator@ update:public:test123@smoothalligator 123
  => data:12779
@smoothalligator@ llookup:meta:public:test123@smoothalligator
  => data:{"createdBy":null,"updatedBy":null,"createdAt":"2022-11-17 21:24:45.496Z","updatedAt":"2022-11-17 21:24:45.496Z","availableAt":"2022-11-17 21:24:45.496Z","expiresAt":null,"refreshAt":null,"status":"active","version":0,"ttl":0,"ttb":0,"ttr":null,"ccd":false,"isBinary":false,"isEncrypted":false,"dataSignature":null,"sharedKeyEnc":null,"pubKeyCS":null,"encoding":null}

But it is being cached when doing plookup

@22easy's secondary server with the cached public key (where the public key has ttr 0)

@22easy@ plookup:test123@smoothalligator
  => data:123
@22easy@ scan test123
  => data:["cached:public:test123@smoothalligator"]

Is this a bug related to this ticket @gkc ? Doesn't ttr=0 mean do not cache?

[gkc]

@VJag do you know answer to @JeremyTubongbanua's question?

[sitaram-kalluri]

@gkc I made a public key on my secondary and it automatically made ttr:0 (so therefore should not be cached when looked up).

@smoothalligator's secondary with public key ttr 0

@smoothalligator@ update:public:test123@smoothalligator 123
  => data:12779
@smoothalligator@ llookup:meta:public:test123@smoothalligator
  => data:{"createdBy":null,"updatedBy":null,"createdAt":"2022-11-17 21:24:45.496Z","updatedAt":"2022-11-17 21:24:45.496Z","availableAt":"2022-11-17 21:24:45.496Z","expiresAt":null,"refreshAt":null,"status":"active","version":0,"ttl":0,"ttb":0,"ttr":null,"ccd":false,"isBinary":false,"isEncrypted":false,"dataSignature":null,"sharedKeyEnc":null,"pubKeyCS":null,"encoding":null}

But it is being cached when doing plookup

@22easy's secondary server with the cached public key (where the public key has ttr 0)

@22easy@ plookup:test123@smoothalligator
  => data:123
@22easy@ scan test123
  => data:["cached:public:test123@smoothalligator"]

Is this a bug related to this ticket @gkc ? Doesn't ttr=0 mean do not cache?

@JeremyTubongbanua: The way caching works for the public key is different from the shared key. TTR does not play a role in caching of the public keys. By design, Public keys get cached on executing the "plookup" verb for the first time. So the above is expected and not a bug.

[VJag]

Yes. We do proactively cache public keys. However, if 'bypassCache' is set to true, plookup will always retrieve the value from the secondary of the owner of the public key.

Syntax for plookup to basspassCache: plookup:bypassCache:true:phone@alice

[JeremyTubongbanua]

Thank you @VJag :)