FWD » Core Development » Database

//define temp-table test field f1 as integer field f3 as integer.

for each test:
 delete test.
end.

do transaction:
    create test.
    test.f1 = 999.
    test.f3 = 2.
    create test.
    test.f1 = 998.
    test.f3 = 5.
end.

do transaction on error undo, leave:
    create test.
    test.f1 = 997.
    test.f3 = 7.
    create test.
    test.f1 = 996.
    test.f3 = 11.
    test.f3 = 13.
    do transaction:
        test.f3 = 17.
        undo, leave.
    end.
end.

find last test.
message test.f3.

define query q for test scrolling.
open query q preselect each test.
get first q.
do while available(test):
  message test.f1 test.f3.
  get next q.
end.

for each test:
    delete test.
end.

I want to point out that my persistent table called test has 2 fields (f1 and f3, both of type integer), but, the field f1 is part of an index (I don't think it matters, but just in case).
The problem seems to be inside the inner transaction when encountering undo, leave. Instead of undo-ing/reverting only the value of test.f3 to 13, the program deletes the whole record of test with test.f1 = 996.

Update: If you replace the middle transaction block with the code below, the program works fine. It reverts only the value of the test.f3, (re)making it equal to 13, and no other places are affected by the undo, leave.

do transaction on error undo, leave:
    create test.
    test.f1 = 997.
    test.f3 = 7.
    create test.
    test.f1 = 996.
    test.f3 = 11.
    test.f3 = 13.
    create test.
    test.f1 = 995.
    test.f3 = 19.
    find prev test.
    do transaction:
        test.f3 = 17.
        undo, leave.
    end.
end.

When you nest transactions like:

do transaction:
   ...
   do transaction:
      ...
   end.
   ...
end.

...this does not represent two full transactions. It represents a full transaction with an enclosed sub-transaction. When OE opens a full transaction, any block further down the stack which normally would open its own, full transaction becomes a sub-transaction block. Think of a sub-transaction as a savepoint, where anything done in that block can be committed or rolled back. If it is rolled back, it does not affect work done outside of it, in the context of the full transaction. Sub-transactions can be nested arbitrarily deeply (there may be a practical limit, but I mean this conceptually).

In FWD, we only open a full database transaction at the outermost, full transaction block we encounter. It stays open and is committed or rolled back when that block ends or iterates. This is not ideal, as SQL best practices encourage transactions to be short-lived, and we don't know how long such a block will take to finish its work, but we follow the 4GL semantic in this regard.

As we encounter sub-transactions within that outermost, open transaction, we use:

• savepoints to represent these sub-transaction units of work on the database side, and
• ChangeSet levels to track the in-memory state of a record.

Both the database and in-memory state need to remain consistent w.r.t. each transaction and sub-transaction level, but they are not necessarily in sync with each other at a given point, because of the timing with which in-memory changes are flushed to the database. So, we use these two independent but related mechanisms.

Session.txCount is the count of full transactions opened by a particular Session instance. The purpose of this variable is only to produce a unique identifier for a particular, full transaction within that scope/context. It is not incremented for sub-transactions. Individual sub-transactions are identified by their level within a given full transaction.

Session.getTxNestingId provides both these values in a 64-bit long. The javadoc for this method is wrong (I must have changed the implementation without updating the javadoc). The method stores the current block's sub-transaction nesting level in the high-order 4 bytes of the return value, and the full transaction identifier (i.e., txCount) in the low-order 4 bytes.

There should only ever be 0 or 1 ChangeSet instances per record at any moment. There is never more than 1, because only one full transaction can be active at a time. So, I believe that part is working correctly. The bug could be in the restoration of state for a sub-transaction, but that is based on intuition only; I have not debugged into your test cases. OTOH, the fact that the results are different in your first test case between temp-table and persistent table use cases suggests some difference in the savepoint processing. Are you using H2 or PostgreSQL as the back-end for your persistent table use case?

Please continue with the investigation, taking my description of the intended design into account, and document your further findings here.

Eric Faulhaber wrote:

...the fact that the results are different in your first test case between temp-table and persistent table use cases suggests some difference in the savepoint processing.

I just realized the results of the 4GL programs are different between these two cases as well.

I think this is explained by the difference in transaction behavior between the temp-table and persistent table use cases.

For the persistent-table case, I believe the for each test loops at the start and end of the program which delete all the records in the table cause an implicit, full transaction to be scoped to the external procedure block. So, the two do transaction blocks are treated as sub-transactions.

For the temp-table case, I believe the delete loops do NOT cause an implicit, full transaction to be scoped to the external procedure block. So, the two do transaction blocks each are treated as full transactions.

You can create a listing of the original program in both forms using the COMPILE statement to confirm this.

Regardless, FWD's results still differ, so we need to understand why.

Thank you for the explanations!
I've taken them on board and I have seen that the bug is in fact about the restoration of the state for a sub-transaction. Unfortunately, it seems that there are more factors to this than just assigning the correct state. I'll use two examples to illustrate this point.

• When entering the full transaction, the record will be given the NEW CHANGED TRACKED state. Also, this means that inside the ChangeSet class, the array states will have CHANGED on the first position.
• When entering the sub-transaction, the state of the record will remain unchanged, but, the array states will have CHANGED on the second position as well.
• At the line with undo, leave, the program would execute ChangeSet.rollback, which means that the record would be given the state DEFAULT.
• When the sub-transaction ends, the program would try to flush the record to the database, which won't work because of the DEFAULT state that indicates that there is nothing to do with the record. This is fine because we do not want to flush the record when the sub-transaction ends.
• When the outer transaction ends, the program would try to flush the record to the database, which won't work due to the same reason: the record's state is DEFAULT. The problem is that the record should've been committed to the database.

if (inner > 0)
{
 liveState.state = states[inner - 1];
}
else
{
 liveState.state = baselineState;
}

• The first two steps remain the same as in case 1.
• At the line with undo, leave, the program would execute ChangeSet.rollback, which means that the record would receive the state CHANGED, taken from the array states. This state corresponds to the full transaction's stored state inside the array.
• When the sub-transaction ends, the program would enter the flush method, but will not commit the record to the database. This happens because a transaction is already active (the full-transaction), and it is good, because this is not the moment to commit. But, inside the Validation.flush method, the record will change its state, becoming CACHED.
• When the outer transaction ends, the program would try to flush the record to the database, but this will not work because the state CACHED does not suggest that it needs flushing.

• The changing of the record's state could be done in two steps (at the moment, it happens only in the ChangeSet.rollback method): first, in the rollback method, make the state equal to DEFAULT so when the flushing part happens (from the sub-transaction), nothing would be done. Then, after the flushing from the sub-transaction, make the state equal to what it is supposed to be. That means that if we are still inside a sub-transaction, make the state equal to states[inner - 1], or, if we are in the full transaction block, make it equal to baselineState. Thus, the commit will only happen at the end of the full transaction, which I think was the intent anyways (with accent on "I think").
• The other solution, perhaps not as "clean" as the other one, is an extension of the second case. The if (inner > 0) would remain inside ChangeSet.rollback, which means that the record's state will be updated (in this case it will become CHANGED). But, inside the flush method, instead of making the state equal to CACHED, maybe another check could be done to see if indeed the CACHED state should be given to the record or maybe another state (or remain unchanged, in this case CHANGED). This way, the state would remain the same until the record is finally committed to the database, and then it would be changed.

I will continue to investigate this, but I just wanted to provide an update. The main test is the same as in the #6923-3, the branch and revision are 3821c, rev. 14361.

Eric Faulhaber wrote:

Are you using H2 or PostgreSQL as the back-end for your persistent table use case?

I am using PostgreSQL to test with persistent tables.

For the persistent-table case, I believe the for each test loops at the start and end of the program which delete all the records in the table cause an implicit, full transaction to be scoped to the external procedure block. So, the two do transaction blocks are treated as sub-transactions.

For the temp-table case, I believe the delete loops do NOT cause an implicit, full transaction to be scoped to the external procedure block. So, the two do transaction blocks each are treated as full transactions.

You can create a listing of the original program in both forms using the COMPILE statement to confirm this.

I have done so. In my findings, the for each loops at the start and end do not affect the full transaction scope, so those two do transaction blocks are not treated as sub-transactions. A case where this would happen would be if the program would start with create test. Then, the whole program would be considered a large, full-transaction.
Still, I will provide further updates on this issue.

I committed 6923a/16047 containing more or less the change in #6923-7. However, I refined it to better consider sub-transaction "gaps" (the ones in which the record was not changed), so that the most narrow sub-transaction with a valid state change will be used. If none is found, the baselineState is honored.

I fine-tuned the clear of PK for _temp so that is cleared based on the baselineState. However, I acknowledged a potential issue when the baseline state has the DMO as NEW, but the starting one isn't NEW. In this case the DMO should reclaim its PK that it had before the rollback. Otherwise, the next created DMO will pick that up and conflict. This is what I am now working on - requiring FWD-H2 changes.

• I made the H2 changes to "reclaimPrimaryKey" if the DMO was NEW in the outer sub-transaction, but got flushed in the inner sub-transaction. In this case, the record was rollbacked from the physical database and its id was lost. Thus, I am calling "reclaimPrimaryKey" to re-reserve the PK as the DMO is back in the NEW state.
  • This was particularly problematic as the DMO changes are rollbacked before the actual savepoint rollback. So my reclaimPrimaryKey was nullified anyway. For this, I moved the in-memory DMO rollback after the physical rollback. I couldn't spot any dependency upon the database state when rollbacking the data and state of the DMO, so I assume it is safe to do so.
• Even with this approach, I encountered an issue that the SAVEPOINT is done after the actual INSERT statement. So my rollback wasn't properly removing the INSERT anyway. This is what I spot in #6923-10 ("Note to self:"). In my example I had tt.f2 = 2. done first so I can force the lazilySetSavepoints to kick in. But if I remove the tt.f2 = 2. statement, the SAVEPOINT is done after INSERT :/
  • To handle this, I created a BaseRecord.touch that quite aggressively prepares the change-set and registers the DMO as undoable. I am calling this before each persister.insert and persister.update to ensure the lazilySetSavepoints(false); is called properly.
• Even with all these changes, there seem to be a quite peculiar code being executed in Validation.flush:
  

  -      SavepointManager.PersistenceCode lastBlockSetup = null;
  -      if (!dmo.checkState(NOUNDO))
  -      {
  -         // preemptively set lazy savepoints in the savepoint manager before we create a "micro" savepoint
  -         // here, because otherwise, the save here will trigger the savepoint manager to lazily create them,
  -         // and we do not want those savepoints nested within this one
  -         SavepointManager savepointManager = session.getSavepointManager();
  -         if (savepointManager != null)
  -         {
  -            // this is a no-op if we are not in a transaction or we are in a full transaction block
  -            lastBlockSetup = savepointManager.lazilySetSavepoints(true);
  -         }
  -      }

• DMOs get rollbacked correctly if simply flushed in a sub-transaction that is UNDOed prior to 14025.
• In trunk, DMOs are not rollbacked if simply flushed in a sub-transaction that is UNDOed because the savepoint is set after the insert/updated.
• With my changes, this is still the case because I pursued another approach with BaseRecord.touch that indirectly calls lazilySetSavepoints. However, because there is a lastBlockSetup triggered after my changes, they are ineffective.

Thus, I am inclined to drop my effort with BaseRecord.touch and revert trunk/14025 (within 6923a). From my understanding #6548 was the culprit, but the example in #6548-3 is no longer relevant because FOR EACH block isn't rollbacking on off-end anymore. So it is not reproducible with/without 14025. In fact, the reason why we removed the rollback on FOR EACH block off-end was exactly because of another example like #6548-3 (part of the refactoring of FOR EACH blocks to embed the query in the block definition rather than calling next within the Body). This decision will require Ovidiu's review.

With all these changes in place, I manage to skip the conflicting DMO on same id issue, but some unit tests I have still fail some assertions due to incorrect field values. Maybe the rollback is now too wide. Continuing investigation.

Committed xfer/testcases rev. 1792 with my unit tests for this. "safe" suites are ones that are cut-out in terms of post-processing to avoid indirectly relying on dirty-share functionality. All "safe" suites pass.

I have reviewed the changes in 6923a/r16049. I think the code will work. Thank you for adding comments with the changes. That made my job much simpler.

A new direct-access API to reclaim a PK. This is used for rollbacked records that were flushed, but reverted back to NEW. Constantin, please review

I would like to have the FWD-H2 changes reviewed. It would be easier to release this and then handle 6923a separately. I have done tests with it on top of 8388a and fixes most of the regressions in #8388.

Constantin, just a reminder. Please review 6923a_h2/60. I want to merge it to FWD_H2 trunk and eventually make a release. It get quite messy to test 8388a otherwise (in which I integrated 6923a).

Alexandru, how is reclaimPrimaryKey supposed to be used?

With 6923a_h2/60, we leave the table indexes in an inconsistent state, when reclaimPrimaryKey is used: for example, a call to truncate will fail, as ArrayList<Row> removed = ((MultiplexedScanIndex) scanIndex).removeAll(session, multiplexId.getInt()); will contain this 'reclaimed' row.

Another example would be if you use the scan index to iterate, you will see this 'reclaimed' row.

I think if a row is added to the scan index, then all indexes need to see this row.

Alexandru, how is reclaimPrimaryKey supposed to be used?

I can agree that my implementation was quite aggresive. But I want it to be used in the following scenario.

• record is created (claiming a new key)
• a sub-transaction is started
• record is flushed in a sub-transaction (full row is updated)
• sub-transaction is rollbacked
• at the end:
  • with trunk, the newly created record doesn't have a PK reserved. Creating another record in the meantime will "steal" its recid.
  • with 6923a, I forcefully claim back the PK because the record is no longer in the DB, but it needs its key to be reserved.
    • this happens in ChangeSet.rollback if the state is not-NEW, but the baseline state is NEW.

I think if a row is added to the scan index, then all indexes need to see this row.

My code is mostly like getNextPrimaryKey which is used when creating a new empty row. In my case, I know what PK I need, so no need to be the NEXT one.
When we create records, we do not update all tree-indexes. We just reserve the PK. This is exactly what I want to achieve.

Alexandru Lungu wrote:

• record is created (claiming a new key)

OK, so this adds the record to the multiplex index

• a sub-transaction is started
• record is flushed in a sub-transaction (full row is updated)
• sub-transaction is rollbacked

What happens at this step in FWD, when the sub-tx is rollbacked? I assume the record is removed completely from the H2 table, including the multiplex index? And this is happening because the flush was done via an INSERT and not UPDATE in H2?

Also, you are missing assigning the multiplex ID to the row, reclaimPrimaryKey.

Also, in other words, after the sub-tx was rolled back: the newly created record still survives, and needs its PK to still be reserved, right? So we are at the state when a sub-transaction is started.

Constantin Asofiei wrote:

Also, you are missing assigning the multiplex ID to the row, reclaimPrimaryKey.

But we don't seem to do this even for getNextPrimaryKey.

What happens at this step in FWD, when the sub-tx is rollbacked? I assume the record is removed completely from the H2 table, including the multiplex index? And this is happening because the flush was done via an INSERT and not UPDATE in H2?

Yes. On flush it was INSERT-ed and on rollback it is simply removed.

Also, you are missing assigning the multiplex ID to the row, reclaimPrimaryKey.

I don't think this is right. The new row should be empty, aka null value for data. This is what a reservation means. If I update the first column (recid) or second column (multiplex), then it will be accounted as a flushed row.
BTW, getNextPrimaryKey(Session, int) doesn't do that.

Also, in other words, after the sub-tx was rolled back: the newly created record still survives, and needs its PK to still be reserved, right? So we are at the state when a sub-transaction is started.

Yes.

Thanks, then I have no other concerns for 6923a_h2 . The changes are additive and can be merged to fwd-h2 trunk (and even FWD trunk). Let me know how you want to proceed.

Thanks, then I have no other concerns for 6923a_h2 . The changes are additive and can be merged to fwd-h2 trunk (and even FWD trunk). Let me know how you want to proceed.

I merged 6923a_h2 to trunk as rev. 60. Please make the release (note that you should downgrade to Java 8 before building to preserve the minimum support).

Alexandru Lungu wrote:

Thanks, then I have no other concerns for 6923a_h2 . The changes are additive and can be merged to fwd-h2 trunk (and even FWD trunk). Let me know how you want to proceed.

I merged 6923a_h2 to trunk as rev. 60. Please make the release (note that you should downgrade to Java 8 before building to preserve the minimum support).

fwd-h2 rev 60 is on projsrv01, can be accessed via build.gradle version change.