FWD » Core Development » Database

1. def temp-table ttbb field f1 as int.
2. def temp-table ttbb no-undo field f1 as int.

They will have both the same DMO, with noUndo = true annotation. The SQL will be created from the DMO, and not from the TemporaryBuffer.define - thus both will be no-undo!

This patch tries to solve it:

Index: rules/include/common-progress.rules
===================================================================
--- rules/include/common-progress.rules    (revision 4897)
+++ rules/include/common-progress.rules    (working copy)
@@ -12023,7 +12023,8 @@
                                      'xml-node-name',
                                      'serialize-name',
                                      'before-table',
-                                     'after-table')
+                                     'after-table',
+                                     'no-undo')
          </rule>
          <rule>tmpTabCrit.put(prog.temp_table, tmpTabProps)</rule>
          <rule>tmpTabCrit.put(prog.work_table, tmpTabProps)</rule>
Index: rules/schema/fixups.xml
===================================================================
--- rules/schema/fixups.xml    (revision 4897)
+++ rules/schema/fixups.xml    (working copy)
@@ -798,7 +798,7 @@
             <action>execLib('annotate-boolean', 'serialize-hidden', 1)</action>
          </rule>

-         <!-- convert serialize-hidden AST to an annotation -->
+         <!-- convert serialize-name AST to an annotation -->
          <rule>type == prog.kw_serialzn
             <rule>parent.type == prog.properties
                <action on="true">level = 2</action>
@@ -806,6 +806,16 @@
             </rule>
             <action>execLib('annotate-string', 'serialize-name', null, level)</action>
          </rule>
+
+         <!-- convert no-undo AST to an annotation -->
+         <rule>type == prog.kw_no_undo
+            <rule>parent.type == prog.properties
+               <action on="true">level = 2</action>
+               <action on="false">level = 1</action>
+            </rule>
+            <!-- leave it at the PROPERTIES, as that is what it is used to emit the NO-UNDO at the DMO -->
+            <action>copy.getAncestor(level).putAnnotation('no-undo', true)</action>
+         </rule>

          <!-- convert xml-data-type AST to an annotation -->
          <rule>type == prog.kw_xml_dtyp

But it contradicts this commit:

revno: 15178 [merge]
committer: Constantin Asofiei <ca@goldencode.com>
branch nick: trunk
timestamp: Mon 2024-04-29 19:30:03 +0300
message:
  Reverted the changes in trunk rev 15170 related to temp-table NO-UNDO emitted at DMO annotation - these must be emitted at the master buffer. Refs #8593 #8363 #8700

So it needs more investigations. I think we need to remove the DMO annotation and use only and only the TemporaryBuffer.define flag, always.

For the structure SQL (create DDL) we can have two sets: one with no-undo and one without. And choose depending if the DMO instance is defined undoable or not.

Constantin, since we will have two temp-tables, with different SQL schema definition (+ transactional at the end), they will resolve to the same SQL table name. We need to add something to the SQL name, to avoid collision, like __nu, or a signature from the procedure name, right?

Teodor Gorghe wrote:

Constantin, since we will have two temp-tables, with different SQL schema definition (+ transactional at the end), they will resolve to the same SQL table name. We need to add something to the SQL name, to avoid collision, like __nu, or a signature from the procedure name, right?

Yes, we will need to have different temp-tables in H2. Lets keep the DMOs unchanged and add the suffix at runtime (so if NO-UNDO, you will have __nu suffix in the SQL name and also the NO-UNDO option at the CREATE TABLE).

The main problem when following this approach is that 4GL allows changing the undoable state by changing the value of UNDO attribute. If we use two distinct SQL tables, it will involve moving the records from one table to another one (at least).

Also, lots of files from persist layer needs to be changed to append __nu suffix.

Teodor Gorghe wrote:

The main problem when following this approach is that 4GL allows changing the undoable state by changing the value of UNDO attribute. If we use two distinct SQL tables, it will involve moving the records from one table to another one (at least).

You mean after you have created and prepared the dynamic temp-table? Or even for a static temp-table?

Also, lots of files from persist layer needs to be changed to append __nu suffix.

Is this because of the dependence on the DMO SQL name annotation? IMO we should get rid of that, too, and let it be calculated at runtime.

Constantin Asofiei wrote:

You mean after you have created and prepared the dynamic temp-table? Or even for a static temp-table?

I mean for dynamic temp-tables. The alternative for that is a H2 change which marks the multiplex as undoable / no-undo.

Is this because of the dependence on the DMO SQL name annotation? IMO we should get rid of that, too, and let it be calculated at runtime.

Yes.

Argh, Alex, UNDO option is modifiable for the lifetime of a temp-table!

def temp-table tt1 field f1 as int.

create tt1.
tt1.f1 = 10.
release tt1.

def var htt as handle.
create temp-table htt.
htt:create-like("tt1").
htt:temp-table-prepare("rtt1").
htt:undo = true.
message  htt:undo.

message temp-table tt1:undo. // yes

temp-table tt1:undo = false.

message temp-table tt1:undo. // no

Teodor: go ahead and fix this as needed. So, DMO annotation for undoable is still not needed. And the undoable state must be tracked, live, at the multiplex ID in H2, and not at the CREATE TABLE DDL. Correct?

Right, that is what I am thinking.

Constantin Asofiei wrote:

Argh, Alex, UNDO option is modifiable for the lifetime of a temp-table!
[...]

Apparently documentation states this. But it can be changed outside a transaction, or within a transaction if the table is empty.

Alexandru Lungu wrote:

Constantin Asofiei wrote:

Argh, Alex, UNDO option is modifiable for the lifetime of a temp-table!
[...]

Apparently documentation states this. But it can be changed outside a transaction, or within a transaction if the table is empty.

Looks like this is already implemented in AbstractTempTable.setCanUndo.

Please create a H2 branch as ~/secure/code/p2j_repo/fwd-h2/active/11745a_h2/

• Make NO-UNDO flag to be multiplex-wise and updatable.

Teodor Gorghe wrote:

Also, there is a preexisting bug in trunk, regarding shared temp-table lookup, which requires changes in the conversion and runtime to add a new TemporaryBuffer.useShared(String, Class, boolean) overload. Should I proceed fixing that?

Please post a test for this.

Also, make sure the tests are converted to ABLUnit and committed to tests/table/ in the testcases project.

Please post some details why you've decided to still keep the table-level NO-UNDO in H2.

Constantin Asofiei wrote:

Please post some details why you've decided to still keep the table-level NO-UNDO in H2.

I don't have an reason why I have kept table level H2. Should I remove it and rely only by multiplex UNDO?

Teodor Gorghe wrote:

Constantin Asofiei wrote:

Please post some details why you've decided to still keep the table-level NO-UNDO in H2.

I don't have an reason why I have kept table level H2. Should I remove it and rely only by multiplex UNDO?

Yes. We don't want two H2 tables for the same DMO. And it will make things clearer.

Alex - should we leave in H2 the NO-UNDO option at the table, and we'll just not use it from FWD? It will add some small complexity in H2, but I don't think is different from what Teodor has already done in 11745a_h2 branch.

I have found a regression of 11745a which I am fixing right now. Also, I am adding more tests regarding UNDO flip and per multiplex undo in tests/persistence/temp_table/undo_state/.

Strange error found during testing:

   │  ├─ deleteThenUndoLeaveKeepsDeletion ✘ ** Requested to persist DMO of type com.goldencode.testcases.dmo._temp.Ttmut_1_1__Impl__ with recid 6656. A different DMO instance of type com.goldencode.testcases.dmo._temp.Ttmut_1_1__Impl__ is already bound to this session with the same recid

With the testsuite which I have make, there are 22 tests which are failing (from 102 tests), but with 11745a, which is not committed yet, the number of failing tests has decreased just to 2. The remaining failures are related to error handling, we doing raise an error when we need to.

I have already removed the table-wise undoable flag on H2, but I am waiting for Alex's answer.

Do you have trunk 16704 for #11632? There we tried to solve a similar problem.

If you have rev 16704, try disabling session reclaiming. We need to figure out the root cause.

• Added persistence/temp_table/undo_state tests.

I ran with trunk rev 16706, with the session reclaiming disabled, but the issue still occurs.
Failing test is TestNoUndoTempTableMutations:deleteThenUndoLeaveKeepsDeletion.

Alex - should we leave in H2 the NO-UNDO option at the table, and we'll just not use it from FWD? It will add some small complexity in H2, but I don't think is different from what Teodor has already done in 11745a_h2 branch.

It is fine by me. NO-UNDO is a syntax that can be avoided, that is all. There is a very small parsing overhead ... very small.

Alexandru Lungu wrote:

Alex - should we leave in H2 the NO-UNDO option at the table, and we'll just not use it from FWD? It will add some small complexity in H2, but I don't think is different from what Teodor has already done in 11745a_h2 branch.

It is fine by me. NO-UNDO is a syntax that can be avoided, that is all. There is a very small parsing overhead ... very small.

Actually, after discussing with Teodor, I think we need to remove it - it just adds another layer of complexity, as in H2 we will need to consider the table-level NO-UNDO.

│ ├─ deleteThenUndoLeaveKeepsDeletion ✘ ** Requested to persist DMO of type com.goldencode.testcases.dmo._temp.Ttmut_1_1__Impl__ with recid 6656. A different DMO instance of type com.goldencode.testcases.dmo._temp.Ttmut_1_1__Impl__ is already bound to this session with the same recid

If this is reproducible with trunk, please let me know. We see such issue at some customers and we are not able to reproduce. If you have a test that reproduces, it is a gold-mine.

Yes, it is very reproduceable with the latest trunk and the 11745a state which I currently have, fixes that issue.

Yes, it is very reproduceable with the latest trunk and the 11745a state which I currently have, fixes that issue.

Perfect! I will watch this task if you need further help from me. I think we shall have 11745a delivered asap to customers; let me know how to help fast track it.

Do you have trunk 16704 for #11632? There we tried to solve a similar problem.

Constantin, AFAIK, the customer in question has force-no-undo-temp-tables flag in directory.xml, which make all tables no-undo. Do this also happen in their docker deployment?

Alexandru Lungu wrote:

Constantin, AFAIK, the customer in question has force-no-undo-temp-tables flag in directory.xml, which make all tables no-undo. Do this also happen in their docker deployment?

Yes, both projects have this in directory.xml

Then this task won't help any of that projects, right? Unless 11745a has other changes that affect reclaiming for no-undo.

That test still fails with trunk, because the expected result is different, because the temp-table is undoable, but it doesn't fail with the session recid conflict error.

Teodor Gorghe wrote:

That test still fails with trunk, because the expected result is different, because the temp-table is undoable, but it doesn't fail with the session recid conflict error.

Please find a detailed explanation for the Requested to persist DMO of type. At the least, we will understand why it happened and keep it in mind if we see it in other places.

Constantin Asofiei wrote:

Teodor Gorghe wrote:

That test still fails with trunk, because the expected result is different, because the temp-table is undoable, but it doesn't fail with the session recid conflict error.

Please find a detailed explanation for the Requested to persist DMO of type. At the least, we will understand why it happened and keep it in mind if we see it in other places.

COMPLETELY AGREED!

• The DMO interface is declared to be undoable, but the definition of temp-table is no-undo (I have another testcase which uses the same DMO, but it is really undoable).
• According to the trunk state, the undoable flag from DMO is used to create the SQL, so H2 thinks is an undoable table
• FWD server-side uses UndoableProvider, which is set from the TemporaryBuffer.define, which in this case, is no-undo.
• By having this state mismatch, the second test method fails because it is expected to keep the records in the table (trunk rollbacks the records when it is not supposed to be).
• 4GL reclaims PKs. Since H2 thinks it is an undoable table, on the next create, it will assign the PK to the first PK from the batch because the whole batch was reclaimed.
• Session cache old record wasn't evicted yet because you know, FWD knows this is a no-undo table, which is an optimization to don't evict records which is supposed to be in the temp database. Reaches to that error when associating to the session.

In one of the projects, there is a setCanUndo(true) call, even if it has force-no-undo-temp-tables; but this should be caught by TemporaryBuffer.isUndoable.

Although I think in trunk we may have an existing problem in DynamicTablesHelper.generateSchemaAst - it uses the table's no-undo status, and not via TemporaryBuffer.

      if (!builder.isUndoable())
      {
         // TODO: if other properties must be added, extract this node outside if
         ProgressAst propsAst = new ProgressAst(new CommonToken(ProgressParserTokenTypes.PROPERTIES,
                                                               "properties"));
         tableSchema.addChild(propsAst);

         propsAst.addChild(new ProgressAst(new CommonToken(ProgressParserTokenTypes.KW_NO_UNDO, "no-undo")));
      }

This should no longer be a problem in 11745a, right?

I have removed that section in 11745a.

Constantin, I have committed 11745a/16705 and 11745a_h2/67 (they match).

Please take a look.
I will also commit the remaining tests. I have found another issue in FWD, but it is unrelated to 11745a. I am committing the new tests and investigate the issue. It is related to the fact that a dynamic buffer for a temp-table defined as no-undo, but behind, is treated as undoable.

Added more tests in testcases about temp-table undo: logslogs

added tests/persistence/temp_table/undo_state
added tests/persistence/temp_table/undo_state/MixedFlavorsNoUndo.p                                                                                               
added tests/persistence/temp_table/undo_state/MixedFlavorsUndoable.p
added tests/persistence/temp_table/undo_state/NoUndoRowsAfterUndo.p
added tests/persistence/temp_table/undo_state/NoUndoScopeReopen.p
added tests/persistence/temp_table/undo_state/SharedUndoCalleeCount.p
added tests/persistence/temp_table/undo_state/SharedUndoCalleeFields.p
added tests/persistence/temp_table/undo_state/SharedUndoCalleeN.p
added tests/persistence/temp_table/undo_state/SharedUndoCalleeU.p
added tests/persistence/temp_table/undo_state/SharedUndoCallerN.p
added tests/persistence/temp_table/undo_state/SharedUndoCallerRows.p
added tests/persistence/temp_table/undo_state/SharedUndoCallerU.p
added tests/persistence/temp_table/undo_state/TestDynamicTempTableUndoAttribute.cls
added tests/persistence/temp_table/undo_state/TestDynamicTempTableUndoDefault.cls
added tests/persistence/temp_table/undo_state/TestDynamicTempTableUndoFlip.cls
added tests/persistence/temp_table/undo_state/TestMixedUndoFlavorsShareOneDmo.cls
added tests/persistence/temp_table/undo_state/TestNoUndoSurvivesRepeatedScopes.cls
added tests/persistence/temp_table/undo_state/TestNoUndoTempTableMutations.cls
added tests/persistence/temp_table/undo_state/TestSharedTempTableUndoConflict.cls
added tests/persistence/temp_table/undo_state/TestStaticTempTableUndoAttribute.cls
added tests/persistence/temp_table/undo_state/TestSuite.cls
added tests/persistence/temp_table/undo_state/TestUndoAcrossProcedures.cls
added tests/persistence/temp_table/undo_state/TestUndoBlockForms.cls
added tests/persistence/temp_table/undo_state/TestUndoRecordIdentity.cls
added tests/persistence/temp_table/undo_state/TestUndoStateRedeclaration.cls
added tests/persistence/temp_table/undo_state/TestUndoableTempTableMutations.cls
added tests/persistence/temp_table/undo_state/UndoCalleeCreate.p
added tests/persistence/temp_table/undo_state/UndoCalleeCreateAndUndo.p
added tests/persistence/temp_table/undo_state/UndoCalleeCreateNoUndo.p
added tests/persistence/temp_table/undo_state/UndoProcDriver.p
added tests/persistence/temp_table/undo_state/UndoableRowsAfterUndo.p
added tests/persistence/temp_table/undo_state/zfile_set.txt
Committed revision 1874.

The failing test described in last note is TestUndoStateRedeclaration.SecondBufferOnNoUndoTableKeepsTheRow, FWD should not drop the record with id 2.

Debugged into the test and the issue is related on how we flush temporary buffers when changing a field, inside transaction.

For temporary records, there is no dirty-share because they get immediately flushed to temp database on every change, except for one case: inside a transaction. Reference: TemporaryBuffer.isAutoCommit, Validation.validateMaybeFlush.
There is an issue on that: when a rollback happens, like undo, leave in this case, gets into RecordNursery.transactionEnded and newRecords/updatedRecords gets cleaned up. Nothing wrong related with undoable records, but it is an issue with no-undo records. RecordNursery forgets that it has a pending new/updated record, which triggers a missing/stale record on the next FOR EACH/FIND query.

This is what happens in the TestUndoStateRedeclaration.SecondBufferOnNoUndoTableKeepsTheRow.

Initial fix for this I think is to change the TemporaryBuffer.isAutoCommit is in transaction to be gated by the fact that the buffer is undoable.

There is an issue on that: when a rollback happens, like undo, leave in this case, gets into RecordNursery.transactionEnded and newRecords/updatedRecords gets cleaned up. Nothing wrong related with undoable records, but it is an issue with no-undo records. RecordNursery forgets that it has a pending new/updated record, which triggers a missing/stale record on the next FOR EACH/FIND query.

Mind that there is a (huge) change on #8388 about proper handling of UNDO in regard to RecordNursery. Any test that fails due to invalid state of records regarding dirty-share / RecordNursery due to UNDO is going to be fixed by #8388. Lets have the effort here (#11745) stick to the UNDO/NO-UNDO segregation of temp-tables with same schema.

Ok, got it, but I have documented the issue here.

Teodor: please get #8388, apply your changes over that branch and check the tests. It may be something very specific to no-undo temp-tables.

I have applied the changes over 8388a and ran my test suite. The test from #11625 has been fixed, but the testcase described in #11745-52 not.

I have applied the changes over 8388a and ran my test suite. The test from #11625 has been fixed, but the testcase described in #11745-52 not.

What is the effect of this bug? I am inclined to prioritize 8388a merging and this task merging to fix multiple customer potential issues. The issue found in note 52 is empiric, but I want to understand how prone is to occur at a customer site. Is it a "Requested to persist DMO of type" error? If so, I prefer to have it fixed, but on top of 8388a and 11745a changes (because these two change already quite a lot on the persistence layer and any fix should be implemented on top).

Teodor Gorghe wrote:

Initial fix for this I think is to change the TemporaryBuffer.isAutoCommit is in transaction to be gated by the fact that the buffer is undoable.

Yes, TemporaryBuffer.isAutoCommit is now !persistenceContext.isTransactionOpen(). But a NO-UNDO temp-table must also be auto-commit. Is changing this to consider NO-UNDO temp-tables enough?

No, it is a different issue.
The recid conflict was fixed in this branch and the full description is in #11745-45.
The issue described yesterday is another issue, which is not fixed yet. I don't think it will cause recid conflicts because H2 knows about it and doesn't generate conflicting recids. So, just a phantom/leaking record in the H2.

• Table.setMultiplexNoUndo - this part can pose problems; if the entire table is no-undo, the someone trying to disable no-undo for a multiplex must not be allowed - I would fail there with SQLException than just return false.
  

     public boolean setMultiplexNoUndo(int multiplex, boolean noUndo) {
        // the whole table is no-undo, so no multiplex of it can deviate
        if (isNoUndo) {
           return false;
        }
  

• Alex - please also review 11745a_h2.

Table-wise no-undo is now set only when force-no-undo-temp-tables is enabled.

When that flag is disabled, it will use the setMultiplexNoUndo, with no undo flag on table-wise set to false.

Teodor Gorghe wrote:

Table-wise no-undo is now set only when force-no-undo-temp-tables is enabled.

But in H2 terms, someone trying to break the contract of the SQL-level no-undo state IMO must abend. I'd like to have that protection explicitly to avoid any possible mishap.

Constantin Asofiei wrote:

Teodor Gorghe wrote:

Table-wise no-undo is now set only when force-no-undo-temp-tables is enabled.

But in H2 terms, someone trying to break the contract of the SQL-level no-undo state IMO must abend. I'd like to have that protection explicitly to avoid any possible mishap.

Ok, I understand, I am making this to throw error and display in the logs on FWD server as SEVERE.

About 11745a: the only issue I see is about bufferType, which gets recreated in RecordBuffer.rebuildBufferType.

BufferType is used as a key in bufferManager.changeScopes. Switching undo state loses this information.

Done the setMultiplexNoUndo abend in 11745a_h2/r68.

Alex - please also review 11745a_h2.

The changes look right to me. The critical invariants I want to make sure we respect:

• We don't change the no-undo flag in the middle of a transaction. All of the implementation relies on having the same state within a transaction, so an incorrect toggle of the no-undo flag while a transaction is being processed is fatal.\
• We don't leak records. This was the first and most important thing to keep in mind when working with this index. We had many times regressions on load testing due to records leaking (not being rollbacked correctly, recids not being dropped, multiplex not being closed, empty batches remaining, etc.)
• Recid management is a complete PITA. NO-UNDO does reclaim keys eagerly while UNOD reclaims keys after transaction ends. This is because an eventual UNDO can revive records that need to reclaim their original recid, so no new records can take that recid until the full transaction is committed. UPDATES make this a bit tricky because they are implemented as DELETE + CREATE, so CREATE should get the same recid as the one dropped by DELETE. Of course, the ROLLBACK of an UPDATE carries the same responsibility.
• There are some quirks regarding recid reclaiming. For instance, 4GL seems to store the records in pages and reclaiming happens only within the last page (or something like that if I recall correctly).
• FWD implements INSERT INTO SELECT FROM for COPY-TEMP-TABLE. Of course, such operations should respect recid, undo, and other invariants.
• FWD implements bulk DELETE of temp-tables. Of course, each individual delete of a bulk delete shall be registered to undo log and be sensitive to undo, recid reclaiming and other invariants.

Most importantly, we need (many serious tests) for each of the bullets above. The memory leak stuff shall be tested (either with heap dumps or debugging). We had in the past some memory leak tests (stress tests of many many hours) that failed due to leaks. We don't have access to them AFAIK anymore, but we can investigate on small tests.

Please also check other tasks that relate to FWD multiplexed index, recid reclaiming, etc. Make tests for them and retest in the original customer applications to ensure they are not regressed.

Constantin Asofiei wrote:

About 11745a: the only issue I see is about bufferType, which gets recreated in RecordBuffer.rebuildBufferType.

BufferType is used as a key in bufferManager.changeScopes. Switching undo state loses this information.

Done in 11745a/r16706.

Teodor Gorghe wrote:

Done in 11745a/r16706.

Thanks. Let me know when you want 11745a_h2 to be merged to fwd_h2 so we can updated 11745a/build.xml and also publish the new fwd_h2 rev.

Right now, I am looking for the previous H2 issues, to create testcases to test for leaks or regressions, but I will tell when testing is done.

For the issue in #11745-45, do we postpone this in a different task/branch? I don't want to leave it as is.

Constantin Asofiei wrote:

For the issue in #11745-45, do we postpone this in a different task/branch? I don't want to leave it as is.

Sorry, I mean #11745-51, the SecondBufferOnNoUndoTableKeepsTheRow - does this test still fails?

still fails.

Teodor Gorghe wrote:

still fails.

Does changing TemporaryBuffer.isAutoCommit to consider undoable state fix it?

Constantin Asofiei wrote:

Teodor Gorghe wrote:

still fails.

Does changing TemporaryBuffer.isAutoCommit to consider undoable state fix it?

Yes, it fixes it.

Teodor Gorghe wrote:

Constantin Asofiei wrote:

Teodor Gorghe wrote:

still fails.

Does changing TemporaryBuffer.isAutoCommit to consider undoable state fix it?

Yes, it fixes it.

Why not include this in this task? As is not related to #8388.

I brought up #8388 because it embeds the fix for #6923 which is a massive rollback rework. If it is unrelated, I am ok to fix separately.

Done the autocommit change in 11745a/r16707.
Also committed rev 69 in the H2 branch because I have found a small bug and a behavior in 4GL which we need to match.

About #11745-66, these commits should address the bugs found during testing.
I have made a test suite which creates lots of records, scenarios which flips the UNDO state, create/delete and updates, etc. I have took a heap dump before the run and after the run (while the session is alive), and I don't see any record which was being leaked into the H2. Only these are the only relevant retained instances:

2259:             1             64  org.h2.pagestore.db.MultiplexedScanIndex$Batch
2424:             2             48  com.goldencode.p2j.persist.BufferManager$BatchModeData
7420:             1             32  com.goldencode.p2j.persist.BufferType

I am continuing to go through all H2 tasks and make the testcase (example: #10519)

Added more testcases in xfer testcases. I think this now tests all what Alex pointed in #11745-66.
I don't see any obvious issue. Should I continue with testing on all active projects?

Teodor/Constantin: can we have the H2 branch merged and a new fwd-h2 artifact released? This can ease the testing as we can simply plug the released artifact in the project.

From my POV we can move in Internal Test. I will try to deliver some tests as well in the process.

I would have preferred to merge the H2 branch when all the testing is done. I don't want to go into a situation that I find a H2 issue and need to make a new branch while testing a project.

Currently, I have temporary changed the build.gradle to take the H2 artifact from lib.local, to avoid the headaches while patching H2 artifact.

Teodor Gorghe wrote:

Currently, I have temporary changed the build.gradle to take the H2 artifact from lib.local, to avoid the headaches while patching H2 artifact.

Teodor, I can build 11745a_h2 and upload it to our gcd repo as 1.65-trunk, so build.gradle can be changed and target rev 1.65-trunk, and let testing move ahead without lib.client. If all testing passes, then 11745a_h2 can be merged (and re-release as rev 1.65-trunk again).

Ok, I understand.
I'll update the build.gradle.

build.gradle got updated in 11745a/r16708.

I created some tests, committed to testcases/1883:

• tests/persistence/temp_table/undo_state/dmo_identity
• tests/persistence/temp_table/recid_undo_log_compound_ops

They pass both with trunk and 11745a. There is however a test that fails with both: com.goldencode.testcases.tests.persistence.temp_table.recid_undo_log_compound_ops.TestBulkDeleteUndoLog

The tests check if undoing a empty-temp-table is actually re-populating the temp-table. In OE it does, but in FWD it does not. I will let me know if I can have a quick look and check if this is a FWD issue that can be quickly fixed in FWD 11745a. I am worried that such scenario can also trigger DMO recid conflict (a bulk delete that is not correctly rollbacked may mess with recid) which may be the main culprit of #11237. WIP

Ok, might not be related with that test, but still with empty-temp-table, but when the temp-tables are no-undo. May be something which is related to the force undo flag from directory.

Teodor Gorghe wrote:

Ok, might not be related with that test, but still with empty-temp-table, but when the temp-tables are no-undo. May be something which is related to the force undo flag from directory.

This was a very quirky thing. Apparently, EMPTY-TEMP-TABLE is not a method for a temp-table handle. Although it is documented like such, actually running it yields a warning "EMPTY-TEMP-TABLE is not a queryable attribute" (even if the syntax is htt:empty-temp-table(). method-like). I did not spot this because it was wrapped in a no-error clause. In FWD it actually worked at conversion time, so hBulk.unwrapEmptyTempTable().deleteAll() was generated. I rewrote the statement using a buffer receiver and made it work.

This is a bit stressful because I think I spotted empty-temp-table being used on a temp-table handle instead of a buffer handle in customer application. But at the same time I am not certain if this method works only in some OE versions. Teodor/Constantin, can you confirm my speculation (running empty-temp-table() on a temp-table handle actually yields a warning and the table is not cleared)?

Yes, that's indeed true. That happens on OE 11.6.

def temp-table ttA no-undo
    field f1 as int.
def buffer bA for ttA.

temp-table ttA:empty-temp-table.

MESSAGE "done".

buffer ttA:empty-temp-table works, but temp-table ttA:empty-temp-table not.

Teodor: please look into the history of persist.TempTable:

• 018 OM 20190513 Extends EmptyTempTable to access EMPTY-TEMP-TABLE method.

Ovidiu: do you recall why this was added?

The bzr log for revno is 11310.1.8:

revno: 11310.1.8
author: Ovidiu Maxiniuc <om@goldencode.com>
committer: Constantin Asofiei <ca@goldencode.com>
branch nick: 3751a
timestamp: Mon 2019-05-13 19:22:38 +0300
message:
  Extracted EMPTY-TEMP-TABLE in a separate interface and extended both TempTable and Buffer from it. refid: #3809, #3751

Feature #3809 (ProDataSet support) is obvious (cleaning the tables/datasets), but Feature #3751 (implement support for OO 4GL and structured error handling) not. At least not directly.

Well, after more careful analysis I see that the branch was 3751a which explains the reference to #3751.
But why this particular update was committed to 3751a and not to 3809x is still an enigma 🤔.

Alex/Teodor: is empty-temp-table a concern for this task? It may be that we emitted for each tt1: delete tt1. end. via a tempTable(tt1).emptyTempTable() at some point, instead via the buffer. And is just something that got missed. Anyway, we should fix it, but in a different task.

I don't think it is a concern from POV.

I have tried to reproduce the issues from #11237 & #11632, but I wasn't able to. AFAIK, both points to empty-temp-table and deleteAll, with force-no-undo-temp-tables set to true.

I have tried to reproduce the issues from #11237 & #11632, but I wasn't able to. AFAIK, both points to empty-temp-table and deleteAll, with force-no-undo-temp-tables set to true.

I will request directory.xml again for the systems in #11237 and #11632. Maybe the force-no-undo was removed. I recall that we had a talk about it (last year maybe?) and the customer in question had second thoughts about it. So they might have considered testing without it on some environments. Low chances tho.

Alex/Teodor: is empty-temp-table a concern for this task? It may be that we emitted for each tt1: delete tt1. end. via a tempTable(tt1).emptyTempTable() at some point, instead via the buffer. And is just something that got missed. Anyway, we should fix it, but in a different task.

No, but please open a separate DB task. I think this is an easy scenario to miss, especially if you use no-error (like I did). It stresses me that I found several cases in customer source code where a table-handle is actually used with EMPTY-TEMP-TABLE.

Alex, please open a task in the customer's project and ask why that code has h:empty-temp-table(). for a temp-table handle. I just want to make sure there isn't some kind of weird quirk in 4GL.

I will let #11795 drive any decision on empty-temp-table over temp-table handles. From my POV the testing is clear on #11745.
Teodor, is there other testing to be done here?

Started to run conversion and runtime testing on all projects.
For now, 11745a testing has passed on at least 3 projects.

There is an unexpected failure on ChUI regression tests, caused by 11745a changes.
Currently investigating.

Alex, it is a bit unrelated to this task, but the changes for zfile_set.txt on testcases project are intentional (make on rev 1850 and 1883)?

The failure from ChUI regression testing is related to the Paul's fix about the no-undo flag. The issue was already fixed by Octavian, well documented in #3211-293.

I will rebase the branch, rerun the conversion and check if there are any more fails.

Rebased task branch 11745a to trunk rev 16724. Last revision is now 16730.

Redid the conversion for ChUI regression and the SharedVariableManager.addTempTable undoable flag is now correct.
Runtime testing is ongoing.

ChUI Regression testing has passed.

Anything left to test?

4 projects left to test. I will tell when the testing on projects is done.

Didn't documented in the testing plan, but I have asked to Dănuț to run conversion + runtime testing on POC, I don't know how is going on, but I am mostly interested if the conversion passes.

Tested the multi tenant project.
Conversion has differences like

<    tt.Buf tt = (tt.Buf) TemporaryBuffer.useShared("tt", tt.Buf.class, false);
---
>    tt.Buf tt = (tt.Buf) TemporaryBuffer.useShared("tt", tt.Buf.class);

At runtime there is an issues, in the server log I have multiple severe entries like

26/08/28 01:56:16.415-0600 |  SEVERE | com.goldencode.p2j.util.ErrorManager | ThreadName:MSA Worker #24 for app appsrv, Session:00000002, ThreadId:00000009, User:appsrvprocess | In procedure proc.p, shared temp-table tt has a conflict in field, index or undo status. (2075)

this causes some scenarios to fail.
Please let me know if this is cause by a config I missed or a legitimate issues.

Did you used the latest 11745a, including the rebase to the trunk rev 16724?

Used rev 16730 on top of trunk 16724.

Investigating right now. Seems like the problem is on the producer side, SharedVariableManager.addTempTable, and #1752 didn't fixed your case.

Can you upload the conversion (src is only needed) on devsrv01 /tmp, so I can take a look on the converted source code?

Or perhaps, you took the patch and applied over 10614_main.

I used 11745a.

Ok, it is not a false alert. I will wait for the converted sources.

Uploaded ..._src_11745a.zip to tmp. To replicate the issues, just run harness.

Artur, I don't have an explanation why this issue occurs. I know that there is a conflict between two temp-tables with the same definition and that is how it also happens on 4GL if the undoable flag doesn't match.

I don't have the cvt files and the jar just yet, but as I can see on the converted sources, they are right.
Please make sure that you actually applied the converted jar, because using an old jar could also cause this issue.

Yes, that was it, I forgot to replace the application and h2 jars. Retested and it seems fine, sorry for that.