FWD » Core Development » Database

I guess a simple first step is to profile typical flows in real applications and see if we can associate any bottlenecks with operations in the FWD runtime (including H2) related to the tracking and replaying of changes for NO-UNDO temp-tables.

I think Session.log is responsible for logging the sub(tx) details needed for savepoints or rollback. This is enabled only if UNDO_LOG=1, the H2's Session.undoLogEnabled boolean field, but I don't know if this is set via a JDBC URL config or something else.

Anyway, we should benchmark H2 with disabled UNDO_LOG and see what happens. FYI, for a large application, Session.log is being called ~135k times and Session.commit is being called ~75k times.

I am aware of the force-no-undo setting which forces all tables into no-undo. If this flag is set, no SavepointManager is used. Therefore, your scenario is only for those cases where force-no-undo is not used, right?

Therefore, this optimization targets applications which do not use such option. In this case, I will do some preliminary tests on customer applications removing force-no-undo. I will check how much of the time is spent into roll-back + re-roll for NO-UNDO tables.

I think Session.log is responsible for logging the sub(tx) details needed for savepoints or rollback. This is enabled only if UNDO_LOG=1, the H2's Session.undoLogEnabled boolean field, but I don't know if this is set via a JDBC URL config or something else.

It is part of the JDBC URL config and we do UNDO_LOG=0 when using force-no-undo. By the way, this is removed in the latest H2 versions, because it is specific to PageStore.

Anyway, we should benchmark H2 with disabled UNDO_LOG and see what happens. FYI, for a large application, Session.log is being called ~135k times and Session.commit is being called ~75k times.

I guess we need a more granular statistic: how many times a certain type of table (NO-UNDO) is rolled back. I will add a dummy keyword ("NO-UNDO") to H2 and extend the logger to make a distinction between UNDO and NO-UNDO.

Constantin, do you see measurable SQLRedo and/or SavepointManager activity in your profiling of the large application POC? SQLRedo indicates NO-UNDO temp-table work we could eliminate. SavepointManager is less specific, as it could represent regular or NO-UNDO temp-tables, or persistent tables.

Alexandru Lungu wrote:

I tested with a large customer application (~10 mins POC) and I got only 7 redo operations solved in ~20ms in total (lets say ~3ms on average). Even so, that 7 redo ops happened only when I got some logical errors in the application; none of them were spontaneous. This makes sense as the application ensured data consistency by undoing.

This metric alone suggests to me this is not a high priority candidate for performance improvement.

Furthermore, I just added OrmNoUndoTrace in 3821c/rev. 14470. This is used to analyze the overhead of creating SQLRedo operations and storing them in case a redo ever happens. Some of the operations are using BaseRecord.deepCopy. The results for the same scenario as before leads to ~20000 redo operations created and stored in ~60ms (0.003 ms per operation).

This finding is less clear to me. Would you consider this a bottleneck in the context of your hardware, relative to the timings of other operations?

Eric Faulhaber wrote:

Constantin, do you see measurable SQLRedo and/or SavepointManager activity in your profiling of the large application POC? SQLRedo indicates NO-UNDO temp-table work we could eliminate. SavepointManager is less specific, as it could represent regular or NO-UNDO temp-tables, or persistent tables.

SQLRedo is minimal - no impact. Same for SavepointManager.rollback. Only SavepointManager.registerBlockHooks has a count of 66k, and it creates some 150k objects.

Constantin Asofiei wrote:

Eric Faulhaber wrote:

Constantin, do you see measurable SQLRedo and/or SavepointManager activity in your profiling of the large application POC? SQLRedo indicates NO-UNDO temp-table work we could eliminate. SavepointManager is less specific, as it could represent regular or NO-UNDO temp-tables, or persistent tables.

SQLRedo is minimal - no impact. Same for SavepointManager.rollback.

Another data point that suggests this is not a high priority candidate.

Only SavepointManager.registerBlockHooks has a count of 66k, and it creates some 150k objects.

In a mixed environment of regular and NO-UNDO temp-tables, this needs to happen regardless of whether we modify H2 to support NO-UNDO temp-tables natively. Well, something like this needs to happen; there may be a way to optimize it.

In any case, I'd like to get Alexandru's response to #6995-7 before we decide to put this H2 change on the back burner.

Eric Faulhaber wrote:

Furthermore, I just added OrmNoUndoTrace in 3821c/rev. 14470. This is used to analyze the overhead of creating SQLRedo operations and storing them in case a redo ever happens. Some of the operations are using BaseRecord.deepCopy. The results for the same scenario as before leads to ~20000 redo operations created and stored in ~60ms (0.003 ms per operation).

This finding is less clear to me. Would you consider this a bottleneck in the context of your hardware, relative to the timings of other operations?

Hard to say. The SQLRedo instances are created and stored pretty fast for the profiled scenario IMO. I am rather worried about the high number of instances created in the process: ~20k SqlRedo, most of which store a deep copy of a DMO. There may be an impact on the GC that I didn't profiled. Implementing H2 level NO-UNDO removes overhead of storing these operations and deep copies.

Note that these times are highly dependent upon the record size, operation type and frequency. INSERT and UPDATE more precisely use deepCopy (instance creation using reflection, array copy and some bitset work).

If we are doing any substantial work implementing NO-UNDO in the customer scenario, then let's move ahead.

Radu, based on our new FWD-H2 branching system #7058, I've created 6995a_h2 to hold your patch from #6995-13. However, I can't trivially use the patch due to a conflict in src/main/org/h2/command/dml/Update.java. I see that the patch also includes your changes from #6928 (which I think are safe to remove as rev. 6 doesn't have problems with UPDATE).
Please check-out 6995a_h2, redo your patch for rev. 6, apply it over the branch, run ./build.sh test and note my remarks from #7058-6 regarding tests. I am planning to do some performance tests with your changes asap today.

I committed to 6995a_h2, rev.7 the changes that you asked for.
Also, when running build.sh test, the errors were the same as in #7058-6, so I hope that you won't encounter any problems. As always, let me know if this is not the case.

Eric, I added you as a watcher now.

I am not necessarily sensitive with huge chunks of code being removed (less is faster), but should we keep some of the old code for reference? Removing most of SqlRedo and SavepointManager code makes the FWD code-base completely incompatible with any other H2 or FWD-H2 version.

My opinion is that we should not clutter the code with dead stuff.

• Dead code has a real cost. Everyone looking at the code will spend more time on it than needed because there is dead code there. This multiplies over the lifetime of the code, so it can be very expensive.
• We are moving aggressively here to modify H2 and it will be an essential part of our performance solution. We are not going back. Either the H2 maintainers will accept our changes or not. If not, then we maintain our own fork of the project.

Eric has the final say here.

I have not yet reviewed the changes in detail, but my opinion is that removing all SQL redo code from FWD is OK as long as:

• we have the requisite NO-UNDO support within H2 to support that change in FWD; and
• the pair of change sets in FWD and H2 are clearly documented as dependent upon each other and must happen in lockstep; and
• the set of changes in both are scoped only to this issue (i.e., are not intermingled with any larger set of changes).

That way, we can always see in version control what the exact set of changes were to enable this shift.

Eric Faulhaber wrote:

I have not yet reviewed the changes in detail, but my opinion is that removing all SQL redo code from FWD is OK as long as:

• we have the requisite NO-UNDO support within H2 to support that change in FWD; and
• the pair of change sets in FWD and H2 are clearly documented as dependent upon each other and must happen in lockstep; and
• the set of changes in both are scoped only to this issue (i.e., are not intermingled with any larger set of changes).

That way, we can always see in version control what the exact set of changes were to enable this shift.

I agree. Other changes we have for H2 follow the same patters: upgrade FWD with its corresponding H2. My concern is mainly the fact that H2 is not synchronized with FWD "out-of-the-box". Reverting/upgrading to an older/newer FWD means also reverting/upgrading to an older H2 and vice-versa.

Right now, H2 is delivered through GCD maven repository and is currently set at revision 6 (trunk). Because there are several changes/branches of H2 (each one with an artifact), should we expect to pull only trunk H2 builds from the GCD maven repository? If we are to test with a H2 branch, I guess we can just check-out and build. However, we may need to find a way to tell gradle to use a local artifact of H2 instead of the online one. If this is something we need to discuss in depth, I think we can move to #7058.

Radu, please redo the FWD patch considering my first comment in #6995-16 regarding getCreateTemporaryTablePostfix over the new 6129c. Hopefully, the re-patching won't be that complex.

Eric, do you plan to review the FWD/H2 changes from #6995-13? Once 6129c reaches a stable state, we can go with the latest plan from #7058-11.

Code review 6995-6129b.patch and fwd-h2-6995.patch:

Nice work, Radu!

I agree with Alexandru's comments in #6995-16 and as noted previously, I am ok removing all the SQL redo code, under the conditions stated in #6995-19. In fact, I am happy to be rid of all the redo baggage; that was a very awkward requirement that I never liked.

The rest of the changes look fine to me. I am more familiar with the FWD code than the H2 code, so I am relying on your and Alexandru's experience in this area.

Integrated the patch into 7062a. Also, the H2 changes made it to trunk.

• Can't we have both transactional and no-undo create table statements? I see that "transactional" is applied only for !noUndo.
• Please note that transactional doesn't mean that the DML operations over the table are logged to be able to commit/rollback - this is by default. transactional is a keyword added by the FWD team a while ago to make CREATE TABLE a "transactional statement". In the original H2, creating a table commits the existing transaction, creates the table and opens a new one for the upcoming changes. Adding transactional boost the performance to avoid the transaction open/close overhead.

Please consider making "transactional" available for both undo and no-undo tables (in H2 dialect). Also test the H2 parser to be able to parse both transactional and noundo.

UPDATE: Also remove the JMX we no longer use for REDO operation.

Alexandru Lungu wrote:

• Can't we have both transactional and no-undo create table statements? I see that "transactional" is applied only for !noUndo.

I double checked and we can have both. In that case, the function gets called twice, but with different parameters (this is the current manner).

UPDATE: Also remove the JMX we no longer use for REDO operation.

Done. Committed this to 7026a, rev. 14508.