FWD » Core Development » Database

• supporting FIELDS/EXCEPT clause, so we can fetch "incomplete" records (#2137)
• a lead to calculate field lists at conversion times to avoid fetching unnecesarry properties (#6721)
• lazy hydration as expected from #6720

Most probably, #6720 will help a lot the adjacent "lazy hydration" related work through some complex result-set lifecycle management. However, the H2 _temp database can simply benefit from any naive kind of "lazy hydration" due to the fact that the data is already in the memory. My point is that we can avoid hydrating a whole DMO at once just because the data is already somewhere in the memory, but wrapped in a H2 specific representation. There is no need to eagerly deserialize/serialize it into the FWD representation if not needed.

1) Retrieving data from H2 using SQL provides us a result-set containing record snapshots. A JDBC ResultSet is in fact an instance of org.h2.result.LocalResultImpl, wrapping a ArrayList<Value[]> rows.

2) The FWD hydration process traverses all fields (unless FIELD/EXCEPT is specified) and adapts the data representation to BaseDataType. This way we are mostly working with fully hydrated DMOs. This process actually traverses an entry from rows (basically a Value[] instance) and converts from H2 representation (ValueString) to Java representation (String) to FWD representation (character).

3) We are eagerly fetching, so we are always adapting representation for all fields, even if only some (2/3 fields) are going to be used. By now, we can see why #6720 is relevant. However, the H2 _temp database itself is already storing the data in memory, so we are not facing the network and serialization overhead from #6720. We can tackle this naively!

• check the column index is between bounds
• check that the connection, session, statement, result-set are not closed
• check that result-set is not out of out of bounds
• convert to Java representation (rarely, only for data-types like ValueTime, ValueDate, ValueTimestamp and ValueTimestampTimeZone)

• store a reference to the Value[] retrieved by the result-set in TempRecord (lets name it "fallback")
• presume that any TempRecord has initially empty fetchedFields (very similar to readFields)
• override checkIncomplete or create checkFetched, to automatically fetch the data at the specified index from the stored "fallback". Mark it as read. The fetchedFields should always be a subset of readFiels.
• BaseRecord.data is almost fully encapsulated
  • any complex logic can trigger complete eager fetching to ensure consistency with the current implementation. Remove the fallback after complete eager fetching to free memory faster.
  • getData is not that widely used. From my POV, it is best to remove it and encapsulate behavior in BaseRecord.

6) There is a quite big difference to #6720: there is no need to match the result-set lifecycle to the buffer. The row is in memory, so we need just to reference it through H2 direct-access.

• the H2 representation will live as long as the DMO lives. Therefore, there will be a slight memory increase. However, if we do the fallback management responsible enough, we can make this limitation negligible.
• the H2 representation is bound to the TempRecord. So we have an "external" dependency right from the DMO. However, it is basically a composite relationship to the H2 instance, so our DMO will exclusively dominate the snapshot, making it self-contained.

I think point 5 can also apply to #6720 if we use a proper interface. However, there is a way more complex mechanism to keep track of ResultSet lifecycle there, as we don't have direct-access.

Eric, please let me know of your progress in #6720 to avoid having two implementations for lazy hydration. I can match this "H2 direct-access fallback" on your structure if needed. Otherwise, we can go with something similar to point 5..

Danut, please focus on implementing a demo for point 5 - we should have a fast mock to test its performance. Therefore, make sure that SQLQuery.hydrateRecord is actually using direct-access to generate a fallback for the TempRecord to the values provided by the result-set. This way, we don't need to go through all columns and read the properties, but just reference the value array provided by the result-set. We will hydrate when we will access data.

Make sure you use DirectAccessHelper as a gateway to the H2 internals; we should avoid spreading H2 dependencies through the FWD code.

The "fallback" from TempRecord (or more specific, TempTableRecord) should be a higher level representation (an interface) to allow any further work on this area with other means of lazy hydration.
Make sure you hydrate from the fallback when needed. That is, when you access a specific index from data that wasn't fetched yet. Override checkIncomplete for this, at is already spread in all getters.
There is some extra work with data (like in the case of buffer-copy), where you will need to do a complete hydration. Implement such mechanism to do complete hydration and use it when needed. Make sure you don't hydrate the same field multiple times.

override checkIncomplete or create checkFetched, to automatically fetch the data at the specified index from the stored "fallback". Mark it as read. The fetchedFields should always be a subset of readFiels.

This is wrong. I see that readFields is set exactly when a field is hydrated from the result-set, not when required through FIELDS. Therefore, for lazy hydrated records, readFields is empty at the start.
There shouldn't be two bitsets (fetchedFields and readFields) but only one which stores the read fields. Basically, readFields should be enough.

I think you can use fallback only for fully hydrated records for now. It is an overshoot for now to support this with partially hydrated records anyways.

Committed 7185a_h2/rev.14 and 7185a/rev.14528 as part of the initial implementation for lazy hydration.

• Instead of storing a reference to the Value[] from the current row of a ResultSet in TempRecord, I store a ResultSet that only contains the respective row and is obtained through DirectAccessHelper.getJdbcResultSet.
• Instead of fetchedFields, I use the fallback and the BaseRecord.readProperty method to update BaseRecord.data.
• Added a new boolean property to DmoMeta called hasExtentFields, which will be used to avoid retrieving the ResultSet for DMOs that have extent fields.
• I relied on DirectAccessHelper for any operations requiring H2 and avoided importing new packages inside FWD.

Please review the changes.

Review for 7185a/rev. 14528

• In TempRecord:
  • readFields.set(i); is redundant as readProperty already sets the readField.
  • allowsFallback is redundant; you already have a fallback variable, so you allow fallback.
  • Make sure you don't read a property twice. Therefore, use readProperty only when readFields.get(i) is false.
  • Are you sure for (int i = 0; i < data.length + 3; i++) is right? You do readProperty(i, fallback, i + 3);, so you may read for data.length + 2 offset, which is out of bounds. I guess you should just use for (int i = 0; i < data.length; i++). Please double-check.
• In BaseRecord:
  • return this instanceof TempRecord; this is not close to OOP standards. Please override allowsFallback in TempRecord and return true from there; return false from BaseRecord.
• In DirectAccessHelper:
  • Change the direct access error message; use something like "can't extract current row from result-set". I think is better to throw a DirectAccessException instead of IllegalStateException. This will allow a nice recovery to the JDBC/SQL method of hydrating.
• In SQLQuery:
  • If you catch DirectAccessException, just do a SEVERE log and allow the standard JDBC/SQL way of hydration; don't associate and return an invalid record; just try again with the other way of hydrating.

There are many usages of data straight-away that I missed in the first iteration. Please make BaseRecord.data private. Enforce any access to data through getters. getDatum should hydrate eagerly. Don't use checkIncomplete for getDatum, as in some cases getDatum should work with incomplete records. Just ensure you always honor lazy fetching when you do getDatum. Also, do full-hydration when using getData. Replace all direct usages of data with getters (on one element or on all elements).

Please mind the setters. setDatum(i) should be done only after you've hydrated (this is implicit after you do getDatum(i)). Now that I am thinking, we may see a slowdown if we enforce access to the low-level data array through getters. I think this was something we tried to avoid already. Please pend until some further advice.

Eric, Constantin, please advice. If we attempt to implement lazy-hydration in any form (including this naive in-memory technique for H2), we may need to enforce access to data using getters so that we honor the "fallback" (the result-set). Is this something acceptable, as long as we see that data is used directly in so many places (ScollableResults, Persister, Validation, etc.)? There are encounters like dmo.data[offset] null in Validation that should be replaced with dmo.getDatum(offset) null to honor lazy fetching.

Review for 7185a_h2/rev. 14
The changes are meant to support the operation of "extracting the current row from a result-set and wrapping into another dummy result-set"

• Please rename FWDDirectAccessDriver.getJdbcResultSet to FWDDirectAccessDriver.extractRow or something similar.
• There is an implicit cast for ((JdbcResultSet) rs).getResultInterface(). I guess H2 will always use JdbcResultSet as their result-set implementation, but a FWD developer is not constrained to pass any kind of ResultSet implementation there. Please make a type check. If rs is not JdbcResultSet, throw an SQLException. This is more friendly then a ClassCastException.
• Please do a double-check for what can be null or not. I am concerned that ((JdbcResultSet) rs).getResultInterface() may return a null result. Or the current row may be null, as the result-set cursor is out of bounds.

When doing direct-access, it is best to take the safest route, throwing SQLException whenever an unexpected state is reached (something is null, database objects are not found, etc.). The SQLException will be wrapped into a DirectAccessException by DirectAccessHelper. Ultimately, anytime you catch a DirectAccessException, you should log a severe and fallback to the "JDBC/SQL way".

• Fixed TempRecord.
• Made BaseRecord.java private which resulted in several changes to use getters and setters.
• Created setSimpleDatum which is used by BaseRecord to update it's data property directly.
• Renamed DirectAccessHelper.getJdbcResultSet to DirectAccessHelper.extractCurrentRow and made changes to error messages and logs.
• Added null checks in FWFDirectAccessDriver.extractCurrentRow.

Committed 7185a/rev.14530. Fixed a NPE in TempRecord.getDatum() where fallback was assigned but readFields was null.

Alexandru Lungu wrote:

Eric, Constantin, please advice. If we attempt to implement lazy-hydration in any form (including this naive in-memory technique for H2), we may need to enforce access to data using getters so that we honor the "fallback" (the result-set). Is this something acceptable, as long as we see that data is used directly in so many places (ScollableResults, Persister, Validation, etc.)? There are encounters like dmo.data[offset] null in Validation that should be replaced with dmo.getDatum(offset) null to honor lazy fetching.

The idea when I first implemented BaseRecord was to make the data array directly accessible as an instance field to other classes in the package and to subclasses. This was done with performance in mind, knowingly sacrificing encapsulation.

However, at that time, I did not contemplate the complication of lazy hydration, nor other use cases which would make direct access to data less practical. In places where lazy hydration makes that direct access inadvisable, go ahead and do what needs to be done. That being said, I want to be very careful about adding a lot of logic around each access to an element in the data array, which could impact performance negatively. The point of lazy hydration is performance, after all.

Where we can continue to support faster access to the data array, even if is through fast, minimal methods, rather than directly as a field, I want to do that. In other words, just use the lazy-hydration-aware access where absolutely necessary. Hopefully, that doesn't turn out to be everywhere, or if it is, hopefully the cost savings we get from doing that outweighs whatever we are giving up by removing direct access to the array.

Eric Faulhaber wrote:

The idea when I first implemented BaseRecord was to make the data array directly accessible as an instance field to other classes in the package and to subclasses. This was done with performance in mind, knowingly sacrificing encapsulation.

Makes sense.

However, at that time, I did not contemplate the complication of lazy hydration, nor other use cases which would make direct access to data less practical. In places where lazy hydration makes that direct access inadvisable, go ahead and do what needs to be done. That being said, I want to be very careful about adding a lot of logic around each access to an element in the data array, which could impact performance negatively. The point of lazy hydration is performance, after all.

I agree. I didn't have a mean of comparing easy access without lazy hydration vs getter/setter access with lazy hydration. I was curious if we can have a stable implementation so we can continue exploring our options.

Where we can continue to support faster access to the data array, even if is through fast, minimal methods, rather than directly as a field, I want to do that. In other words, just use the lazy-hydration-aware access where absolutely necessary. Hopefully, that doesn't turn out to be everywhere, or if it is, hopefully the cost savings we get from doing that outweighs whatever we are giving up by removing direct access to the array.

• for Record, we have basic getters for getDatum and getData. This falls into "fast, minimal method" category.
• for TempRecord, we have:
  • If fallback exists and the datum is not read, a procedure to read the property. This is time consuming OFC.
  • If the datum was read or the fallback was consumed, a super call; this falls into "fast, minimal method" category with a simple if check.

Based on #7185-14 I committed 7185a/rev.14531. I confirmed that TempRecord.checkIncomplete is redundant and removed it, I've done the same to TempRecord.getFallback because it isn't used anywhere.

Done some tests with latest 7185a/rev. 14531 and latest 7185a_h2:

• FWDDirectAccess.retrieveResponse you created is used now in two use-cases: the former "wrap a row found by unique index in a dummy JDBC result-set" and the new "wrap a row from another result-set in a dummy JDBC result-set"
• Unfortunately, even if these look similar, they bahave differently. In the first case, a row from H2 is provided (so it includes computed columns - beginning with __i or __s). In your case, the provided result-set doesn't have computed columns. This causes an ArrayIndexOutOfBoundsException as it tries to append to the result-set the column i and the value i. However, cols.length != values.length.

Note that the computed columns are marked as invisible in H2. Therefore, you can append a boolean flag to retrieveResponse named valuesHaveInvisible and fix it this way (skip values only if they include invisible).

Committed 7185a/rev. 14532 and 7185a_h2/rev. 16: handle cases when result-set is composed out of multiple row structures. Add an offset from which the row is extracted. Fixed #7185-18,

Rebased 7185a with trunk (rev. 14525). 7185a is now at 14530.

I have troubles testing 7185a. One customer application works flawlessly, while the other in hanging right at the start-up.
I will double-check the changes, in case I missed something critical.

Radu, can you please check-out 7185a and test it against adaptive-scrolling/non-scrolling tests. Note that you should do the local FWD-H2 dev. set-up (to locally load, compile and use 7185a_h2). Ensure the tests are using temp-tables. Please report any regressions you encounter there.
UPDATE: Danut, please retest in-depth the changes on a customer application. If you can't find any issues, consider doing some extensive testing with FWD, especially multi-table compound query optimized into an adaptive query.

Alexandru Lungu wrote:

Radu, can you please check-out 7185a and test it against adaptive-scrolling/non-scrolling tests. Note that you should do the local FWD-H2 dev. set-up (to locally load, compile and use 7185a_h2). Ensure the tests are using temp-tables. Please report any regressions you encounter there.

I've done multiple tests (including parallel runs) over these changes but I could not find any regression.

Committed 7185a/rev. 14531: small fixes. Danut, please review.
I am close to finding the regression we are chasing for some time.

Alexandru Lungu wrote:

Committed 7185a/rev. 14531: small fixes. Danut, please review.
I am close to finding the regression we are chasing for some time.

I am ok with the changes.

I also retested a customer application and found no signs of regressions. Several tests that I had were in the same situation. It is good that you are getting closer to the root of the problem.

Alexandru Lungu wrote:

Danut please review. I will start the profiling session asap. I would like to know the performance change by now and retrieve a statistics of how often is the lazy hydration used and how many times the "retrieve all columns procedure" is triggered. I will post the update here.

I understand the cause of the problem now and I am ok with the solution.

Committed 7185a/rev. 14533:

• Optimized TempRecord.fallback usage. Redone the profile and got to +0.25% performance decrease, comparing to my baseline. This is manageable. It seems that the whole readFields instance creation on copyArray and the lack of readFields.nextClearBit made a difference.
• I am also optimizing FWDDirectAccess.retrieveResponse to avoid creating dummy Column instances for each column in the result-set and use the table's columns instead (lazily filtered if there are invisible/computed columns intertwined). Table.columns collection is immutable, so we shouldn't worry that columns may appear/disappear from the result-set. This way we also optimize #7062 (direct-access). In my solution, there are no O(N) loops anymore (unless JDBC ResultSet columns are searched by name - not something I encountered in FWD). This will be even faster after #7108 is merged, as we shouldn't worry about invisible/computed columns anymore (neither in direct-access nor in lazy hydration).

I have a stable solution, but I am still thinking of fine-tuning it. I am confident we will turn this around to -% performance increase.

Committed 7185a_h2/rev. 19.

It includes optimization for FWDDirectAccess.retrieveResponse:

• If no record is found with direct-access, return a single EMPTY_RESULTS instance. This way we don't create redundant results instance.
• Count the invisible columns in a table: if there are no invisible columns, let the results reference the table columns to retrieve metadata when needed.
• If there are invisible columns, let them be filtered lazily (when needed).
• Make copies of the values/columns arrays only when needed.

Testing with 7185a_h2/rev. 19, I get -0.14% performance improvement. I will retest without the lazy hydration FWD changes, just to check if the FWD-H2 changes alone are more performant. At that point we can decide what to tackle next.

Rebased 7185a_h2 to trunk rev. 15. Not it is at rev. 20.
Committed 7185a_h2/rev. 21 including some extra optimizations. Also included the EmptyResults and LazySingleResults implementations in FWD-H2.

These are the results only with FWD-H2 changes. It is a bit worrying now that rev. 14540 alone is slower then 14529 (by +0.43% without any FWD-H2 changes).
For this task's purpose, the FWD-H2 changes show a consistent improvement alone to the direct-access engine (less instances created, faster invisible column filtering, faster no-record resolution).
I will update this table with the 7185a changes on lazy hydration; from some prerequisite tests, they should provide a slight boost over all.

I updated #7185-30 with the times regarding the FWD changes (lazy hydration). While the FWD-H2 changes provides a consistent improvement for direct-access, the FWD lazy hydration changes introduce a severe slow down (+0.5%~0.6%).
This was a bit harsh to test because of the 7156/rev. 14529 - 14540 time differences, so I had really different results between runs. Even for this +0.5%~0.6% I had runs which were only +0.1%.

My suggestion: as the FWD-H2 changes are robust and well-tested, we can move on with integrating them now. A -0.5% improvement is something we can take as it is for now. I would like to integrate the direct-access support for lazy hydration as-well, even though it won't be used in FWD yet. I am planning to redo the performance tests once we have a new stable baseline.

I will leave this task WIP until then. I am brushing up 7185a_h2 to merge in FWD-H2 trunk tomorrow morning. Danut, please make a quick review on 7185a_h2 before that time.

Review 7185a_h2. The changes are good to go, you can proceed with the merge.

Merged 7185a_h2 into FWD-H2 trunk as rev. 17.
This doesn't require any FWD changes; it just optimizes the direct-access.
The changes for lazy hydration were included, but are not "activated": the API is there, but it is not consumed.

I will do another profiling round next week after stabilizing FWD-H2 and get some customer applications reconverted with the recent RTRIM changes (#7108).

Rebased 7185a to latest trunk. Branch is now at rev. 14671.
Regression testing passes successfully.
Started profiling test. This round will be decisive for 7185a.