FWD » Core Development » Database

Alexandru Lungu wrote:

Basically, allow each session to be reclaimed, but only within a limited time threshold.

I put some thought on this and it isn't that trivial. There is not obvious moment when we can check that this time threshold is passed so we close the session and connection. The best we can do here is to create a separate daemon thread which does the clean-up. Once per second (or more?) it can check if there is an orphan session hanging, so it closes it. However, I don't immediately understand the time overhead of such separate thread. I would like a single thread to manage this cross-session, so we don't have one daemon thread for each FWD user. Hopefully the synchronization won't hurt that much here.

For to start, I think we can bypass this time threshold and just let the sessions be reclaimed without limitation. But bear in mind that a time threshold mechanism should be easily added afterwards.

Danut, please go ahead with a basic implementation of a SessionFactory. It should reclaim the existing session if exists or create another one if none exists. Closing a session means only "markReclaimable", without actually closing it. Please take care about the multi-context environment - let one SessionFactory per Persistence.Context for now. Ensure you do the pre-closing work (rollback orphaned transactions), without actually closing (connection close).

Finally, please note that Eric tried this at some point (in #4021) and didn't got encouraging results. Please double check your implementation and do some debugging / VisualVM / memory dump kind of work after, to check if this solution is OK.

I created a SessionFactory which allows a Session to be reclaimed. I did a round of testing which mimic the specifications mentioned in #7167-1.

If there are any other methods that should be mentioned, please tell me so that I can update it with the snapshot data. The performance of reclaimable cache is very good, currently only one session can be reclaimed since the context holds one session per database.

I remodeled the profiling test to fit a multi-user case. I never detailed what the contents of test are, only that it mimics #7176-1, so I am going to write them down now, including the modifications included in the remodeling.

• Creates 1k records in a persistent table.
• Creates 1k records in a temporary table.
• Using another temporary table, it stores 20 non-empty and 150 empty handles of the persistent table and 1350 empty handles of the temporary table with 1k records.
• Run an external procedure 1.7k times.
• Deletes the records from the persistent table.

• The persistent table has an initial number of 1k records.
• Creates 1k records in a temporary table.
• Using another temporary table, it stores 10 non-empty and 20 empty handles of the persistent table and 100 empty handles of the temporary table with 1k records.
• Run an external procedure 200 times.
• No records are deleted from the persistent table.

The difference in both test sets is not impressive (-5% - +5%) and the only improvement is of 38% when testing a single client (case that was already tested in #7334-3). I will retest after I improve the implementation.

I added the Daemon thread for closing session based on a lifespan value. I also tested different lifespan values as suggested (1s, 5s and 10s). I used the same multi-user test with 1,2,5 and 8 clients. I couldn't test more than 8 clients because it would only open 8 and the rest would start after any of the other clients are closed. The test use the average of a total of 5 runs, but in cases where a run takes longer than 2-3 minutes, I only did 3 runs.

I think the most appropriate value for the lifespan of a session should be between 3-5 seconds. Please let me know if I should try testing using 3 as the lifespan of the session. I also did not prevent registering the same session multiple times because I didn't want to influence the results of the lifespan with a value that is too small or too large. Alexandru, what values should be considered in this case?

EDIT: Test specifications related to pause calls.

Danut, please commit your latest changes to 7334a. I will do a quick review.

Committed 7334a.rev14568. This update allows managing of expired session across all SessionFactory instances using a Daemon Thread.

Review of 7334a.rev14568

• Make initialize() method synchronized to avoid concurrent initalization of the thread.
• This two step hasReclaimable and factory.create is not safe from thread's perspective. You may incorrectly recieve reclaimed as true, even if the create will generate a new session instead. I think it is best to use an "initialization lambda" here. Basically, pass in a lambda (Consumer<Session>), which does the whole associate code after. This will be called by the factory if the session is not reclaimed and will do the whole associating stuff. Avoid synchronizing this lambda call as it is huge. Let the Context do only the bufferManager.registerPersistenceContext after create. Anyway, you should get rid of hasReclaimable.
• Use extends Thread on a separate line.
• You need to evict these factories at some point. When the context is closed, you need to remove the factory from the thread. In otherr words, when a user disconnects, you will need to remove its session facory from the thread. This can be done in Context.cleanup. Ensure you also close the reclaimable session (if any) on cleanup.
• Note that you iterate the factories with a for-each. If you add/remove factories in that time, you may face a concurrent modification exception. Do a for (SessionFactoty factory : new ArrayList(factories)) (or something similar) to iterate a clone of the array.

Danut, can you retrieve the same statistics as in #7334-8 without any pause? I am curios how fast is using a single session in one single demanding run.

Alexandru Lungu wrote:

Danut, can you retrieve the same statistics as in #7334-8 without any pause?

The difference between rev.14566 and rev.14567 is -45.25%. And the one between rev.14567 and rev.14568 is ~1%. There is not much change.

What about multiple client? I suspect your test was done with only one client?

Committed 7334a.rev14569. I applied the changes mentioned in #7334-11.

The implementation shows an improvement for 1 client, but for multiple clients it does not. Since the results are obtained from an average, I observed that in the case of a 5 seconds lifespan, the runs have a minimum difference of 6s and a maximum of 17s between each other when running 8 clients and an average of 3s when running 5 clients. This does not happen when using a lifespan of 10s.

• Double check that each user uses its own session (they have different sessions and connections) and they both reclaim their own session each time needed.
• Check if any client is ever waiting for another client.
• Check the behavior of the session closing thread; does it block any client?
• Other synchronization issues.

Review of 7334a

• Place the opening brace of PersistenceConsumer on the next line.
• I think you can make the code more consistent if you mark expire as synchronized. If the factory is already synchronized, this will be a no-op. However, if somebody uses expire out of context, make sure it uses it properly. Set reclaimable and timestamp to null in finally clause.
• Do a safe check in markReclaimable: if reclaimable is not null, then log a severe and make sure you expire the current reclaimable. You can make the method synchronized, instead of syncrhonizing this. Run a large application and make sure the warning doesn't pop up.
• Delete hasReclaimable
• At lambda definition, wrap the parameter currentSession in parenthesis

Rebased 7334 to latest trunk. 7334a is now at rev. 14578.

Committed 7334a/rev.14579. Applied the changes mentioned in #7334-17.

Danut, as discussed, your tests may be spoiled by the fact that each user is doing lots of small transactions on the same table. Even if NO-LOCK, this still hurts the concurrency of the test-case so that the reclaiming is not that visible in the testcases.

Please use two users and an extra database table named seq with one integer field. It should have a dump with only one row (0). The test starts by each user increasing the first row with EXCLUSIVE-LOCK. This way, the first user gets an even number and the second user gets an odd number. After, each user will make its own scenario. The even user will do transactions on the A table, while the second user will do transactions on the B table. This way, we shall se only the time difference of reclaiming sessions, no locking the same table.

You can do the same test for 5 users (use mod 5, not even/odd) or 8 users (use mod 8, not even/odd), bu t you need more tables.

From my point of view, this is a parallel issue in which multiple users with NO-LOCK on a single table are still slowing each other own. This should be investigated separately (InMemoryLock).

Rebased 7334a to trunk / rev. 14592 7334a is now at rev. 14596

I am doing a test/profiling round with the current changes.

Alexandru Lungu wrote:

Danut, please focus on finishing the tests here. I will like to do some last checks and merge this in trunk.

I remade the multi-user test set after finding out that they locked the same table for a simple record reading. I used what you suggested in #7334-20, tested 2, 5 and 8 clients that access different tables and are routed using another persistent table with EXCLUSIVE-LOCK, here are the results:

Note that the new test is based on #7334-7 multi-user profiling test with the only change being the number of records created in the temporary table which is 10k. Should we consider increasing the lifespan value or leave it as it is?

Eric, Constantin, Ovidiu: can you provide a feedback here. The performance tests show real good results and the implementation looks quite robust. I intend to merge 7334a asap. However, I am not sure of the session time-out policy. Right now, a session expires (and is closed) after 1s of inactivity:

• Should we change the policy to 1s of life-time instead of 1s of inactivity? I am 90% sure that 1s of inactivity is the right policy here.
• Should we make this configurable? This way we can increase time-out period - or even disable it.

Alexandru, there are some concurrency issues, but my main concern is this:

      synchronized (this)
      {
         if (reclaimable != null)
         {
            session = reclaimable;
            reclaimable = null;
            timestamp = null;
            return session;
         }
      }
      session = new Session(database, dmoVersion);
      initialize.accept(session);

If there is no 'reclaimable' session, then the initialize.accept does all the work to associate DMOs with the session. But, otherwise, the session is reclaimed and the DMO state (at session and DmoVersioning) remains unchanged. I think the session cache is OK, but what's the impact on DmoVersioning?

Alexandru, there are some concurrency issues

I will do a second check then. Please let me know if you already spotted something of interest to fix asap. Anyway, this is actually good news; the time may be even better than -2.7% after fixing the issues :)

But, otherwise, the session is reclaimed and the DMO state (at session and DmoVersioning) remains unchanged

From my POV, unchanged is good for this case. I find the #7334 feature similar to "the legacy code doesn't trigger persistence actions for a while, so the Session and DmoVersioning should be unchanged".

I (somehow) see your point. DmoVersioning will not have anything evicted unless the session expires. As long as the session reaches the reclaimable state (not in use, but not expired), all cached records will stay in place both in Session and DmoVersioning. The side-effect on DmoVersioning is that records will be de-registered way later than now, so they will be stored in the DmoVersioning for more time. However, the session will be still linked to the DmoVersioing under the hood, even if not in use.

However, this is quite dependent on the time-out value. The intent here is to use the same session on two scenarios that happen very fast one after another. If the use-case triggers more than 1 work-flow through the FWD persistent layer, we shall execute them using the same database session. This being said, I think we can lower the time threshold even to something like 10ms and still re-use the session between persistence actions.

• This code can execute on the SessionCloseThread. This is not an AssociatedThread (and can't be as there is only one thread) and has no access to context-local data. Any problem during reclaimable.close(); will execute DBUtils.handleException in this thread, which is not OK:
  

     public synchronized void expire()
     {
        try
        {
           reclaimable.close();
        }
        catch (PersistenceException exc)
        {
           DBUtils.handleException(reclaimable.getDatabase(), exc);
        }
        finally 
        {
           reclaimable = null;
           timestamp = null;   
        }
     }
  

• in this code, you need to move initialized = true before the new SessionCloseThread().start();. There is no guarantee the thread will start before the initialize() finishes.
  

        public synchronized static void initialize()
        {
           if (!initialized)
           {
              new SessionCloseThread().start();
              initialized = true;
           }
        }
  

• otherwise, please remove the history numbers and leave only one (the username and date of the change can remain, but we want to keep only one history number per branch).

Constantin Asofiei wrote:

Alexandru, about the concurrency issues:

Danut, please do the fixing.

otherwise, please remove the history numbers and leave only one (the username and date of the change can remain, but we want to keep only one history number per branch).

Sorry for that. I think is the third time you mentioned it :/ Trying to be more rigorous.

I'm not sure how we can solve this:

     catch (PersistenceException exc)
      {
         DBUtils.handleException(reclaimable.getDatabase(), exc);
      }

The intent here is to use the same session on two scenarios that happen very fast one after another.

Think about in reverse: what happens if the LIFETIME is disabled (and the session is 'forever')? Can we get in trouble with the DmoVersioning (I don't mean the cache or memory usage, I mean breaking the application in an abnormal way).

Constantin Asofiei wrote:

if it executes on the SessionCloseThread. We need tests and make sure the Conversation thread for that session gets notified and the exception (and messages) gets processed. Which can be either a DatabaseConnectionException or a StopConditionException, plus the ErrorManager.display calls.

The error management here is quite odd. What if the session closing fails at some point (>10 seconds of lifetime). The user operates the application and at some point the server decides to close the session due to time-out. Thus, the exception will be thrown "out-of-nowhere", uncorrelated to the user action basically.

• do some business logic for ON ERROR? I don't this this is the case, but if it is, it is not quite good, as the session closing is non-deterministic
• display errors to the user + stop the client
  • either the client won't exist at that point, so there is nothing to do
  • the client exists, but most probably it already dealt with some persistence exceptions in the mean-time, or it will face them very soon
• even with all this effort of reporting the problem, the connection will still hang on the database (worst case).

What I am trying to say is that we can't properly recover in case of an exception, so maybe we should just do some logging and let it be. I am looking forward to SessionCloseThread as a FWD internal mechanism, unrelated to 4GL and client layers, so it should just fail in the background.

The intent here is to use the same session on two scenarios that happen very fast one after another.

Think about in reverse: what happens if the LIFETIME is disabled (and the session is 'forever')? Can we get in trouble with the DmoVersioning (I don't mean the cache or memory usage, I mean breaking the application in an abnormal way).

There is something that is not right: what happens when the client actually exits? I think this should be handled so that the session is forcefully closed when the client disconnects, right? So that 'forever' actually means client life-time, not server life-time. Right now, the DmoVersioning and Session will leak in a 'forever' configuration.

Is the 'forever' configuration something that we actually consider (eventually for production environments)? If yes, we will go ahead implementing it.

The agents in an appserver live for the lifetime of the FWD Server - so this Session can exist forever, if this LIFETIME is zero/very large.

What I'm trying to say with this extreme scenario is to think what happens with the DmoVersioning when there are multiple sessions in this 'pending to be reclaimed' state (as they 'live forever'). Don't worry about mem leaks for now: we need to know if this can have an actual impact in the application execution or not (i.e. if we can end up with DMOs in incorrect state, records not saved, etc).

Alexandru Lungu wrote:

Constantin Asofiei wrote:

Alexandru, about the concurrency issues:

Danut, please do the fixing.

• I made the lifespan configurable and is initialized in SessionCloseThread.initialize(). Of course, the initialized variable was moved before starting the thread.
• In #7334-31 you mentioned that we could use logging since it may not be possible to recover if an exception is thrown, so I logged it.
• I modified the history entry numbers, only leaving the first one. Sorry for the trouble.

I also have a question, if we consider that a lifespan with the value zero will make the session to exist forever, is the thread even necessary? No session will expire, unless the factory is deregistered and a reclaimable session exists. I understand that we could use a large value for the lifespan, in this case using a thread makes much more sense, but running the following:

    while (true)
    {
        Thread.sleep(lifespan);
        removeIdleSessions();
    }

what happens when the client actually exits? I think this should be handled so that the session is forcefully closed when the client disconnects, right? So that 'forever' actually means client life-time, not server life-time.

The SessionFactory instance is deregistered in Persistence.Context.cleanup() which I assume is called when the client disconnects. Any reclaimable session available is closed is this situation.

I tested these changes and everything works well, should I commit them?

Alexandru Lungu wrote:

Let me reiterate the protocol here. Constantin, I would like a feedback on this.

• not setting a lifespan means eager session closing (like before).
• setting lifespan on 0 means infinite lifespan (client lifetime). This means we don't need the thread at all.
• setting lifespan on a positive value, means that we need a limited lifespan. This is what we intend to have it done right as it seems the best production use-case.

Yes, this is what I meant. But I didn't mean to actually implement the 'infinite lifespan', if it would not be used in production cases. My goal was to think of possible ways to break the session reclaim support - and, my theory was that if a break point can be found in a 'infinite lifespan' session (never reclaimed), than that bug will exist also in reclaimed sessions.

So, after analyzing DMO versioning the conclusion is that it works OK with the 'infinite lifespan', then we should be good.

Danut, please focus on this task by testing DMO versioning. I am keen on merging 7334a asap.

In addition to #7334-34, make some kind of "infinite testcase". Let it run for 1/2 hours and then use a break-point to analyze the state of Session and DmoVersioning. Keep VisualVM open with the monitor to keep track of any heap spikes of GC overhead. Make your best to create a test with millions of small records that are used by the application (not all in the same time OFC).

I rebased 7334a to trunk. 7334a is now at rev. 14640.

Alexandru Lungu:

Danut, please commit them for further review.

• a negative value: Logs a warning and sets the lifespan to 1s;
• zero: The SessionCloseThread won't run at all (will log it as info), and the factories queue will be initialized in SessionCloseThread.initialize() instead of the constructor;
• positive value: The usual behavior that was targeted in the development until now.

Danut, the best we can do here IMO is to do a multi-session test with clients that read/write to a database. I know you've already tested #7334-20 for performance. Please consider refactoring it with some asserts: ...

I am currently working on creating this test, which is not far from being complete.

I managed to finish and test Session and DmoVersioning, I especially looked into Session.cache, Session.cacheExtension and DmoVersioning.version. The test works by running insert/read/update/delete or a mix of operations on 8 persistent tables, 1 client uses a routing table to run a list of operations on a persistent table in an infinite loop.

I wanted to use the main procedure to run 1 other persistent procedure that kept data about the executed operations but didn't manage to make use of the reclaimable sessions since the logic couldn't be accessed. Since I complicated it a little too much, I ended up discarding the validation and resume to a smaller test.

After 2 hours of using VisualVM and monitoring the activity of the script, I don't see any overhead in the GC. The heap space used follows a pattern of rising for ~5 minutes and then being reduced when the GC takes action. The cache sizes for Session.cache and DmoVersioning.versions are 10k and 80k respectively when running 8 clients.

As for the validation, I plan to use global shared variables and maintain the same logic as much as possible.

Alexandru Lungu wrote:

Danut, just to confirm:

• If I don't set any option, there will be no reclaim.
• If I use 0, the lifetime will be infinite.
• If I use a positive value, this will mean the ms (milliseconds) of maximum time in which a session can be reclaimed.

• If no option is set, the lifespan will be set to a default value (1s).
• If 0 is used as an option, the session will go through the reclaim process indefinitely.
• If a negative value is provided, the lifespan will be set to the default value.
• If a positive value is provided, it will represent the maximum time in which the session can be reclaimed.

In any case, the session will go through the reclaim process.

Alexandru Lungu wrote:

Constantin, if there are other topics to be discussed here, please let me know. I will always check for obvious memory leaks in my profiling tests with 7334a (infinite session lifetime).

I don't have anything else on my mind. Regarding the memory leaks: some of them may be slowly accumulating, and they will not be reported by Eclipse MAT. So please manually check the heap dump, look into the various caches and see if they are abnormal.

Rebased 7334a with trunk rev. 14642. 7334a is now at rev. 14647.

Danut, please update 7334a and try to do a full testing round with Hotel GUI / personal test-cases. Please double check the interaction of your recent bootstrap changes and SessionFactory.SessionCloseThread.initialize();. Ensure that this thread is properly initialized before any session is created / registered / marked as reclaimable.

The time improvement is -2.5%.

I've done two heap dumps and compared them. I have the same number of instances with very similar sizes when comparing, so there is no obvious leak (at least for DmoVersioning, Session, SessionCloseThread and SessionFactory). I had the server running for ~2h with 2 batches of 100 iterations.

Quite interestingly, I have a different number of UnclosablePreparedStatement between dumps (855 and 849, so less in the second dump). In fact, in persistence.orm I have 219.670 instances before and 219.664 after (the difference is related just to UnclosablePreparedStatement).

As it is now, I am planning to merge 7334a into trunk as there are no obvious problems. Danut, please address #7334-34 asap, so we can merge this in the next hour.

Alexandru Lungu wrote:

Rebased 7334a with trunk rev. 14642. 7334a is now at rev. 14647.

Danut, please update 7334a and try to do a full testing round with Hotel GUI / personal test-cases. Please double check the interaction of your recent bootstrap changes and SessionFactory.SessionCloseThread.initialize();. Ensure that this thread is properly initialized before any session is created / registered / marked as reclaimable.

I tested Hotel GUI and my own test-cases. SessionCloseThread is initialized correctly and the process involving creating/registering/marking/reclaiming sessions is proceeding as expected.

Merged 7334a to trunk as rev. 14645. Archived 7334a.

I updated Database Configuration with the configuration and explanation for session lifespan.

- moved to #7425 -

Session reclaiming couldn't be disabled since most values either default to a set value or only disable the managing thread (see #7334-41). In #7490-59, Alexandru requested a way to disable the reclaiming process which was done in 7490a/rev.14683 by using the negative value (when one was used, it would change it to a default value). Database Configuration has been updated with the new configuration settings.

This reached trunk a while ago. Is there anything left on this task? If not, please move it to test.