Project

General

Profile

Support #10222

Is cross-session dirty share available through Fast Find Cache even if disabled?

Added by Alexandru Lungu about 1 year ago. Updated about 1 year ago.

Status:
WIP
Priority:
High
Target version:
-
Start date:
Due date:
% Done:

30%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
production:
No
env_name:
topics:

History

#1 Updated by Alexandru Lungu about 1 year ago

  • Priority changed from Normal to High
The Fast Find Cache is a cache that targets FIND queries. It is cross-session and per-database. If a session does a FIND, then another session will eventually use that "finding", avoiding a database trip. But I am concerned of some transaction aspects:
  • if a transaction does changes, then it is right to clear the fast find cache, otherwise the same session it will still use stale data.
  • if a transaction is committed, then it is right to clear the fast find cache, otherwise other sessions will still use stale data (considering READ COMMITTED transaction isolation)
  • what happens if a transaction did changes before doing a FIND.

My point is that a session can create a record in its transaction and then use a FIND to search for it. The Fast Find Cache will be triggered and the record will be cached in a cross-session cache. If another session does the same find, then the record will leak. Please correct me if I am wrong, but this seems to implicitly support cross-session dirty-share, even if it is disabled.

Currently, the condition is:

         if (ffCache != null                                        &&
             (record == null || record.checkState(DmoState.CACHED)) &&
             cacheResult == null                                    && // NOTE: always null ?
             key != null                                            &&
             !fqlPreproc.isDependsOnSessionAttribute()              &&
             !dirtyCopy)

It is not strong enough to rule out the case when the DMO was newly created / changed by the current session. The dirtyCopy flag is only when the record is found in the dirty-share database, but there are cases when a record could have been already flushed, but not yet committed.

I don't know of any good way to check if a newly created/changed DMO was "committed".
I am setting this as High priority as it leaks records from one session to another.

#2 Updated by Artur Școlnic about 1 year ago

There are two changes we should consider implementing to prevent transaction isolation violations in Fast Find Cache:

1. Add a transaction commit check before writing to the cache:
  • If not in a transaction, cache normally.
  • If in an active transaction that has not yet committed, avoid caching.
  • If in a transaction and the DMO is known to be committed, allow caching.

2. Skip using cached results during uncommitted transactions, to ensure the current session sees only data consistent with its own uncommitted state and to avoid returning stale or conflicting results.

Both changes require a way to determine whether the current transaction is committed. The only transaction API I am aware of is through TxWrapper, but I don't believe P2JQuery has direct access to transactional context.

#3 Updated by Artur Școlnic about 1 year ago

  • Status changed from New to WIP
  • Assignee set to Artur Școlnic

#4 Updated by Artur Școlnic about 1 year ago

  • Status changed from WIP to Review

I added a new dmo state COMMITED, it indicates that a record contains committed data, dafault value is true. The falg should be set accordingly every time a change to the record is detected, as long as the flag is consistent with the actual state of the record, we should not have any more prolems with the ff cache being cross session.
I am using the new flag to determine if a record should be retrieved from and put into the cache.
The code is in 10222a/16012.

#5 Updated by Alexandru Lungu about 1 year ago

After writing #10222-1 I reckon that such change can introduce some performance penalty for same-session case. I one session runs a FIND multiple times, then it won't benefit from the cache considering that the data is changed. I can't tell how this should be fixed. It is something to think about.

#6 Updated by Artur Școlnic about 1 year ago

Alexandru Lungu wrote:

I one session runs a FIND multiple times, then it won't benefit from the cache considering that the data is changed.

Please expand on the idea.
It is my understanding that this is correct behavior, if the data is changed (and uncommitted), it has no place in the cache.

#7 Updated by Alexandru Lungu about 1 year ago

Artur Școlnic wrote:

It is my understanding that this is correct behavior, if the data is changed (and uncommitted), it has no place in the cache.

It shouldn't be leaked to other sessions, but still available to the same-session that did the FIND in the first place. This is what is happening in trunk and is totally OK. It has this side-effect of also leaking. Your fix is also obliterating the possibility for a session to see its own changes - which is a reasonable scenario.

#8 Updated by Artur Școlnic about 1 year ago

So to be clear, in the same session, If a record is modified, but not yet committed, it should be cached and be found in the cache?

#9 Updated by Alexandru Lungu about 1 year ago

Yes, this is what happens in trunk and is a valid scenario. Removing this will be functionally OK, but we will see a performance downgrade.

#10 Updated by Artur Școlnic about 1 year ago

I agree, in a hypothetical scenario where someone changes a record and immediately attempts to find it more than once, this will cause a performance downgrade. Still, maybe we can work around it and allow leaking only within the same session. I will attempt to solve this.

#11 Updated by Greg Shah about 1 year ago

Artur Școlnic wrote:

I agree, in a hypothetical scenario where someone changes a record and immediately attempts to find it more than once, this will cause a performance downgrade. Still, maybe we can work around it and allow leaking only within the same session. I will attempt to solve this.

This is a very common case because there is no cost in OE for this. Some of our customer's applications are very dependent upon this behavior.

#12 Updated by Artur Școlnic about 1 year ago

I made the change, the code is in 10222a/16013.

#13 Updated by Artur Școlnic about 1 year ago

define temp-table tt1 field f1 as int field f2 as char.

create tt1.
assign 
tt1.f1 = 1
tt1.f2 = "1".

create tt1.
assign 
tt1.f1 = 2
tt1.f2 = "2".

release tt1.

find tt1 where tt1.f1 = 1. //fetches from temp-table memory 
find tt1 where tt1.f1 = 1. //no op? record already in buffer
find tt1 where tt1.f2 = "1". // searches again due to a different where

release tt1.
find tt1 where tt1.f1 = 1. // fetches from temp-table memory 

find tt1 where tt1.f1 = 2. //compares the existing record, fetches from temp-table memory 

Is this accurate?
If so, I don't understand how the find can be a no cost operation in OE, if the buffer is released, the find has to fetch the record from the db, same as FWD.

#14 Updated by Alexandru Lungu about 1 year ago

Is this accurate?

find tt1 where tt1.f1 = 1. //no op? record already in buffer

Not really. We can't tell if the record inside the buffer is the one matching the where clause. I doubt OE is doing such short circuits.

FWD: the FF cache shall be hit.
OE: fetches from temp-table memory

find tt1 where tt1.f2 = "1". // searches again due to a different where

Yes.

find tt1 where tt1.f1 = 1. // fetches from temp-table memory

FWD: shall use fast-find cache.
OE: fetches from temp-table memory

find tt1 where tt1.f1 = 2. //compares the existing record, fetches from temp-table memory

FWD: shall use fast-find cache.
OE: fetches from temp-table memory; I doubt OE invests time into short-circuiting here by comparing the current record with the WHERE clause.

If so, I don't understand how the find can be a no cost operation in OE, if the buffer is released, the find has to fetch the record from the db, same as FWD.

I think Greg was not considering temporary database per se. In OE and FWD the temporary database is in-memory and quite fast (with FWD having a certain overhead that was reduced over time). The big problem is the persistent database, where FWD has to pay the price of TCP/IP, TLS, serializing, deserializing data, etc. OE persistent database works with shared memory that is referred as "no cost" by Greg.

#15 Updated by Alexandru Lungu about 1 year ago

PS: FWD is trying to do its best to cover such "no cost" with the FF cache.

#16 Updated by Artur Școlnic about 1 year ago

I used the temp table for simplicity, the behavior of the find should be similar. I am just trying to understand why fetching a record from the database in OE is a low cost operation. AFAIK, the shared memory architecture is used only when the application and database server are deployed on the same machine, otherwise the same network communication protocol overhead is payed in OE.

#17 Updated by Greg Shah about 1 year ago

Artur Școlnic wrote:

I used the temp table for simplicity, the behavior of the find should be similar. I am just trying to understand why fetching a record from the database in OE is a low cost operation. AFAIK, the shared memory architecture is used only when the application and database server are deployed on the same machine, otherwise the same network communication protocol overhead is payed in OE.

Yes, exactly. That is why almost all 4GL applications use this shared memory design. It is so important to them, that Progress forced strongly encouraged all their customers to refactor their GUI applications so that all data access is done in appserver agents and the GUI code just calls the appserver and gets back serialized temp-tables. Doing that is faster than using TCPIP to access the database directly from GUI code!

#18 Updated by Alexandru Lungu about 1 year ago

  • reviewer Alexandru Lungu added

#19 Updated by Alexandru Lungu about 1 year ago

  • % Done changed from 0 to 30
  • Status changed from Review to WIP
Review for #10222:
  • I don't understand session.getCached(dmo.getClass(), cacheResult.getRecordID()).equals(dmo)). This is always true because there is only one single DMO instance in a session.
    • The dmo is retrieved from cache (i.e. session.get(cacheResult)) and the getCached does the exact same look-up. So I can't tell how this can ever return false.
  • Also, unfortunately the logic of simply checking the COMMITTED here may not be enough, because you can't tell if session A's DMO representation is committed or not from the POV of session B.

The tricky part that was overlooked here is that: there are multiple DMO representations across the server and at most one in a single session. Thus, the DMO state is not visible cross-session! Even if you mark it with COMMITTED, that DMO state won't be visible to the other sessions to understand that it is safe to be picked up from the ffcache. It is relevant intra-session to acknowledge whether it is safe to be cached. But this will raise back #10222-7.

The more I think about it, the more I think this is in fact a really hard algorithm to put in place. The most problematic scenario is that:

  • lets say there are two records A and B.
  • first session picks A, changes it to C, does a FIND FIRST and finds B. This is put in ffCache. B is not changed, so we have no hint that this FIND was relying on uncommitted stuff!
  • second session does a FIND FIRST and finds B. As it is clean, it will presume that it is safe to use.

There is no moment when first or second session even acknowledge that the change on A altered the results. This being said, I think fast-find cache is rarely safe to be used cross-session if cross-session dirty-share is disabled. In fact, if a table was touched in a transaction, then the ffCache on that table can't be used (queried or updated) till the end of the transaction. Of course, if we allow leaks in between sessions, than fast-find cache is completely safe to use because it honors the per-index visibility of records provided by the OE quirk.

TL;DR ffCache should be split in cross-session and intra-session caches:
  • cross-session is available only to sessions that did not altered the specified tables in the current transaction in any-way. This constraint can be removed if cross-session dirty-share is activated (obsolete).
  • intra-session is always available.

This effort can be conducted in two distinct efforts (one to create intra-session caches and one to improve the cross-session one). We can discuss this at the daily meeting today.

#20 Updated by Alexandru Lungu about 1 year ago

Greg, I decided in today's stand-up to put #10222 on hold. The implementation of fast-find cache is complete and sound in regard to the original OE behavior (cross-session leaks of records). Although we disable it from directory.xml, fast-find cache will still leak records because it is a cross-session cache. The fix for this is to make the fast-find cache per-session, but that will certainly put a down-grade in the performance. Especially when talking about #9802, that effort relies on adjusting the cache sizes to be able to let "fast-find cache" to work like a cold-time optimizer: if one client runs some scenarios, then other clients will have their FINDs warmed.

There is also a more sophisticated fix to have both an intra-session and cross-session version of ffCache, whereas the cross-session one is populated only with safe data. Safe data comes only from read-only transactions. But I am most concern of the fact that we will invest some non-trivial time on #10222 so that the final result will hopefully be no perf. drawback or functional issues. In other words, there is no "real problem" to be fixed here, rather bring in consistency with "no record leakage cross-session".

In other words, FWD is not leaking anything except FINDs that were done on other sessions as well. It sounds quite bad to me. Overall, I am writing this hoping that you will disagree and we should in fact spend time and fix this :)

#21 Updated by Alexandru Lungu about 1 year ago

Of course, I am looking further for Ovidiu's input on this matter.

#22 Updated by Greg Shah about 1 year ago

FWD is not leaking anything except FINDs that were done on other sessions as well. It sounds quite bad to me. Overall, I am writing this hoping that you will disagree and we should in fact spend time and fix this :)

I agree this is bad and must be fixed.

Eric needs to decide when this gets fixed.

#23 Updated by Ovidiu Maxiniuc about 1 year ago

Alexandru Lungu wrote:

Of course, I am looking further for Ovidiu's input on this matter.

I had a look over 10222a and, indeed, I do not fully agree with the changes. To be short:
  • as Alex noted, the test at line 4522 in RAQ is irrelevant;
  • the COMMITTED flag looks handling does not look right.

To identify whether a record from shared cache is mine, the cache should register the owner and at the moment of lookup that should be compared.
Maybe a possible solution is to have a mix of private FFCs and common FFC. The latter will keep only pristine data which will be copied or otherwise shared to private FFCs?

#24 Updated by Alexandru Lungu about 1 year ago

Maybe a possible solution is to have a mix of private FFCs and common FFC. The latter will keep only pristine data which will be copied or otherwise shared to private FFCs?

I agree with this approach. Though I think simply duplicating entries in caches shall be fine. The session should first look into his private FFC. If is doesn't find any entry, then check the pristine shared FFC.
As for updating the cache, it will choose to update the pristine one or local one depending whether it made changes or not to that table already in the current session.

Still, I must note that the READ_COMMITTED transaction isolation should be honored. Theoretically, any commit will invalidate all FFC (on the changed tables), right? Otherwise sessions won't be able to consider the new committed modifications from other sessions. So in other words:

  • private FFC lifespan will be the same as session's lifespan.
    • Updating should be done only if the target index is changed.
    • Reading should always be done first on this FFC, on a specified index.
    • Invalidation occurs when the current session makes changes to a corresponding index or another session commits changes on the corresponding table.
  • shared FFC lifespan will be the same as server's lifespan:
    • Updating should be done only if the target table is not changed.
    • Reading should always be done after the look-up in the private FFC fails.
    • Invalidation occurs when any session commits changes a corresponding table.

I must note that per-index visibility is no longer relevant when cross-session dirty-share is disabled for the shared FFC - only for the private FFC.

Also available in: Atom PDF