Project

General

Profile

Bug #10542

Write trigger called when multiple buffers reference the same record

Added by Eduard Soltan 10 months ago. Updated 9 months ago.

Status:
Test
Priority:
Normal
Assignee:
Target version:
-
Start date:
Due date:
% Done:

100%

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

Related issues

Related to Database - Bug #9896: Record reference count should account for dirty records New

History

#1 Updated by Eduard Soltan 10 months ago

I have found the following test case. Please note that I defined on book4 table a WRTIE TRIGGER that just message write book:

output to "out.txt".
for each book4:
    delete book4.
end.

create book4.
book4.f1 = 'first1'.
book4.f2 = 'second1'.
book4.f3 = 'third1'.
release book4.

create book4.
book4.f1 = 'first2'.
book4.f2 = 'second2'.
book4.f3 = 'third2'.
release book4.

do transaction:
    define buffer bBook4 for book4.
    find book4 where book4.f1 = 'first1'.
    book4.f2 = 'something'.

    message book4.f1 book4.f2 book4.f3.
    message "first".

    find first bBook4 where bBook4.f1 = 'first1'.

    message bBook4.f1 bBook4.f2 bBook4.f3.
    message "second".

    find first book4 where book4.f1 = 'first2'.

    message book4.f1 book4.f2 book4.f3.
    message 'third'.
end.

The output in OE:

write book
write book
first1 something third1
first
first1 something third1
second
first2 second2 third2
third
write book

In the test case I start by loading in the main buffer the first record of book4 table. Then change on of its fields. Then I load in a secondary bBook4 buffer, the same record.

Then in the main buffer I try to load the second record from book4 table. As you can observe the WRITE is not triggered at this moment. My intuition teals me that it will trigger only when the last buffer that reference the same record gets release. (or an actual RELEASE happens).

#3 Updated by Alexandru Lungu 10 months ago

Then in the main buffer I try to load the second record from book4 table. As you can observe the WRITE is not triggered at this moment. My intuition teals me that it will trigger only when the last buffer that reference the same record gets release. (or an actual RELEASE happens).

This is absolutely true in 4GL. In FWD, this holds due to "multiply referenced" counter. RB.setCurrentRecord does:

boolean singleLoadedBuffer = (currentRecord == null || !currentRecord.isMultiplyReferenced(false));

Based on this it will eventually call write trigger and flush.

Just a heads-up: in FWD, the newly created records are flushed to the database if all indexes are validated (check RecordNursery, makeVisible).

#4 Updated by Alexandru Lungu 10 months ago

Just a heads-up: in FWD, the newly created records are flushed to the database if all indexes are validated (check RecordNursery, makeVisible).

This is done to favor primary database instead of dirty database and avoid the trickiness and caveats of dirty-share (at the cost of incorrectly calling WRITE). I can agree that for functionality, this shall be removed and records shall get into dirty instead. I attempted this before, but it regressed quite a lot (exposing all kinds of dirty issues we had) :/ Please confirm if this is your scenario.

Just a remark on #10542-1, there is only one full transaction and the records are basically NEW within the transaction. Is this right in regard to #10068? I would expect that the data to be created in a separate transaction than the actual flow (and the records shouldn't be actually NEW).

#5 Updated by Alexandru Lungu 10 months ago

  • Related to Bug #9896: Record reference count should account for dirty records added

#6 Updated by Eduard Soltan 10 months ago

Alexandru Lungu wrote:

Just a heads-up: in FWD, the newly created records are flushed to the database if all indexes are validated (check RecordNursery, makeVisible).

I think my test case is much more simpler that that. I do perform a search on the index (f1 field), however update happens of a non-index field. (bBook4 will still find the updated record!?).

The stack of write trigger execution is:

P1.write(Book4$Buf) line: 40    
P1.write(Buffer) line: 17    
DatabaseTriggerManager.fireTrigger(DatabaseEventType, TriggerData, Buffer, Buffer) line: 1719    
DatabaseTriggerManager.lambda$trigger$2(DatabaseEventType, TriggerData, Buffer, Buffer) line: 1015    
0x00007b4e94b6bb40.run() line: not available    
DatabaseTriggerManager.guardedExec(Runnable) line: 2212    
DatabaseTriggerManager.trigger(DatabaseEventType, Class<Buffer>, Buffer, Record, String) line: 1015    
RecordBuffer.maybeFireWriteTrigger() line: 8500    
Validation.flush() line: 633    
Validation.validateMaybeFlush() line: 373    
RecordBuffer.validateMaybeFlush(Record, boolean, boolean) line: 11557    
RecordBuffer.flush(boolean) line: 7257    
RecordBuffer.flush() line: 7204    
FindQuery(RandomAccessQuery).prepareBuffer() line: 4285    
FindQuery.prepareBuffer() line: 1330
FindQuery(RandomAccessQuery).executeImpl(Object[], LockType, boolean, boolean) line: 5557    

Looking at RecordBuffer.maybeFireWriteTrigger(), I didn't find any stopping condition that will take into account the number of references of the record in question. Or anywhere else up the stack.

#7 Updated by Alexandru Lungu 10 months ago

I am inclined to say that prepareBuffer should have taken isMultiplyReferenced into consideration.

#8 Updated by Ovidiu Maxiniuc 10 months ago

@Alex: prepareBuffer() calls buffer.flush(). The flush() (validateMaybeFlush() actually) should NOP if the record is also loaded in another buffer? That would, indeed, require a call to isMultiplyReferenced() to detect this.

#9 Updated by Eduard Soltan 10 months ago

Ovidiu Maxiniuc wrote:

@Alex: prepareBuffer() calls buffer.flush(). The flush() (validateMaybeFlush() actually) should NOP if the record is also loaded in another buffer? That would, indeed, require a call to isMultiplyReferenced() to detect this.

I think in this case we should know if flush was called from query initialization, or from a RELEASE statement (in this case the WRITE should actually be triggered).

#10 Updated by Ovidiu Maxiniuc 10 months ago

Does it matter? In both cases, the modified record is still loaded in a buffer. The trigger will have the chance to fire when the last buffer holding it is released or when the transaction ends.

Please test this assumption to confirm or infirm my assumption.

#11 Updated by Eduard Soltan 10 months ago

Ovidiu Maxiniuc wrote:

Please test this assumption to confirm or infirm my assumption.

I have the following testcase:

do transaction:
    define buffer bBook4_1 for book4.
    define buffer bBook4_2 for book4.
    find book4 where book4.f1 = 'first1'.
    book4.f2 = 'something'.

    message 1.

    find first bBook4_1 where bBook4_1.f1 = 'first1'.

    message 2.

    find first bBook4_2 where bBook4_2.f1 = 'first1'.

    release bBook4_2.
    message 3.

    find first book4 where book4.f1 = 'first2'.

    message 4.
end.

And output in OE:

1
2
write book
3
4

Notice that the WRITE is triggered on RELEASE of bBook4_2 buffer, even book4 and bBook4_1 still hold the same updated record.

#12 Updated by Ovidiu Maxiniuc 10 months ago

Very well.
Then we have to really fire the trigger only on explicit releases. The implicit query initialisation will fire it only if the buffer is the last one holding the record.

#13 Updated by Alexandru Lungu 10 months ago

Then we have to really fire the trigger only on explicit releases. The implicit query initialisation will fire it only if the buffer is the last one holding the record.

  • This happens as protected void release(boolean undo, boolean allowWriteTrigger, boolean honorMultipleReferences) has a flag honorMultipleReferences which is set to true for converted release and false for internal releases.
    • this is done to avoid flush()
  • Note that RB.validate already has some logic to honor isMultiplyReferenced.
    • this is done to avoid flush()
  • The same happens when loading another record in the buffer setCurrentRecord.
    • this is done to avoid flush()

The whole point is that isMultiplyReferenced is always done somewhere before calling flush. But I reckon there are plenty of other places where flush is called without checking isMultiplyReferenced (e.g. RB.create()). This is because flush actually means FLUSH :) And technically you can flush if a record is multiply referenced (i.e. RELEASE case).

#14 Updated by Eduard Soltan 10 months ago

  • Status changed from New to WIP

Committed on 10542a, rev. 16160.

#15 Updated by Eduard Soltan 10 months ago

  • Status changed from WIP to Review
  • % Done changed from 0 to 100
  • reviewer Alexandru Lungu added

#16 Updated by Alexandru Lungu 10 months ago

I am OK with the changes.

  • setLocked should always be set to false. It is basically related to the fact that the buffer is going to be loaded - it doesn't relate to the actual flush.

#17 Updated by Eduard Soltan 10 months ago

Alexandru Lungu wrote:

I am OK with the changes.

  • setLocked should always be set to false. It is basically related to the fact that the buffer is going to be loaded - it doesn't relate to the actual flush.

Fixed review issue in rev. 16161, can it be moved to Internal Test?

#18 Updated by Alexandru Lungu 10 months ago

  • Status changed from Review to Internal Test
  • Assignee set to Eduard Soltan

#19 Updated by Constantin Asofiei 9 months ago

  • Status changed from Internal Test to Merge Pending

This can be merged after 10501a

#20 Updated by Eduard Soltan 9 months ago

  • Status changed from Merge Pending to Test

10542a, was merged to trunk rev. 16238 and archived.

Also available in: Atom PDF