FWD » Core Development » Runtime Infrastructure

Currently, the connection for the landlord database is only made once, during server startup. If something that determines the loss of this connection happens, there is no way to re-establish it without restarting the server. Moreover, in the current state, the connection is open "forever", instead of closing it when things are done and re-opening it when necessary. The first point of this task is to address the resiliency of the landlord database connection.

In terms of the second point, in TenantManager, statements are auto-committed when making changes to the landlord database. The intention is to re-evaluate this approach and add explicit transactions where needed.

Refs: #6229-401.

The initial commit contains only the "connection resiliency" part of the task: now the tenant manager will try to reconnect on a lost connection to landlord database.

Currently, I am working on making the operations transaction-based, which represent the remaining half of the task. I am optimistic to finish this and commit in a few hours.

Ovidiu, I think something is missing. Either I did something wrong when checking out the branch, either there are some changes that were not included in the last commit. For example, on my version of 9002a rev. 15880, in TenantManager.addTenant(), there are unknown references to db.getC3p0MaxStmts() and the other c3p0 functions. I looked into the TenantDatabaseDescriptor class, but I couldn't find them. So, assuming that I didn't do something terribly wrong, will you please do a second check to make sure that there are no compilation errors in 9002a? Thank you!

Thank you, Radu for the attempt.
The explanation is that r15880 of 9002a is 'built' on top of 9831a (which is also in Review and I was expecting it to get into trunk by the time this was done).

Ovidiu Maxiniuc wrote:

Thank you, Radu for the attempt.
The explanation is that 15880 to 9002a is 'built' on 9831a (which is also in Review and I was expecting it to get into trunk by the time this was done).

So, it's alright if we wait for 9831a to reach trunk first, and then you will rebase 9002a after, and then we'll review this branch, right?

Radu,
I have updated 9002a with latest changes. Some of them are already in trunk, some pieces of code had to updated since this branch was not maintained, some rebase 'artefacts' had to addressed.

At any rate, please do a review of 9002a / r15910. It is based on the latest trunk and not other secondary branch.

BTW, I did a basic GUI navigation and a harness smoke test and they did not show any regression.

• In getConnection(), when the first connection is opened, it has conn.setAutoCommit(!trans);. Shouldn't this happen for the second attempt, too?
• In initialize(), the log message in the finally block was not updated. It should have close instead of connect.
• In deleteDatabaseFromTenant(), I think the log messages (4 in total) should be something like Could not delete the database of the tenant due to.... As it is now, it looks like one tries to delete the tenant and not the database.
• At the getConnection() javadoc, the end of the second line should be If it is instead of In it is.

Other than that, I think it's fine. It's good to hear that testing went well, as well.

Branch 9002a was merged to trunk rev 15927 and archived.

• to the landlord database schema;
• to any tenant-related API request or response (including input or output payload);
• requiring configuration modifications (e.g., to directory.xml);
• which would impact the server starting or any functional aspect without first making changes to those items in the previous bullets.

If it does make any such change, we need to roll it back ASAP. Any such change would be considered a breaking change, in that there are devops implications for customers with multi-tenancy systems deployed. We would need to give these customers a chance to review and prepare for the impact of such changes before they used trunk/15927 or later.

• instead of creating a connection to database in initialization, a pool is used. Every time an SQL operation is necessary one connection is acquired and released when finished. There are no configurations for this process, I rely only on default values, but they can be changes in the future, if needed. In fact, most of the time, a single connection will be used;
• each non-trivial statement is executed in a micro-transaction, which are committed or rolled back on errors. The landlord schema is unchanged and the queries are just the same;
• the exposed interface stays the same, there are no changes in the semantics of the public methods the REST API uses.

Therefore, the update is safe.

Thank you.

Did you set the status back to Internal Test only to reflect that the new trunk revision has not yet been pushed? If so, please set it to Test once Alex pushes the release tomorrow.

Infor testing now that all three RedMine issues are in place with the directory.xml removal of breakAfterAcquireFailure
https://proj.goldencode.com/issues/9002
https://proj.goldencode.com/issues/9736
https://proj.goldencode.com/issues/9831

Results are as expected with no signs of the Server failing and API’s are responsive after the PostreSQL connection is reestablished.
The Admin API allows for a login and retrieval of existing Tenants with the PostreSQL connection down.
Once PostgreSQL is started again Admin API and the Server Agents reestablish connections and API calls are successful as expected.