Project

General

Profile

Bug #9036

Multi-tenancy: refactor the database in Persistence context-wise

Added by Radu Apetrii almost 2 years ago. Updated 5 months ago.

Status:
Closed
Priority:
High
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:
Multi Tenant

Persistence.zip (46.5 KB) Ovidiu Maxiniuc, 09/13/2024 08:16 PM

History

#1 Updated by Radu Apetrii almost 2 years ago

Initially, the database field in Persistence was not context-related. However, when implementing multi-tenancy support, the database has become part of the Persistence$Context. Neither of these is the correct approach (in all cases) in a multi-tenant scenario, and, depending on the logic that is using the database, we need to determine which type needs to be used. At first sight, there are around 100 calls, either directly accessing the database, or via Persistence.getDatabase(), but there could be map keys or other structures that involve that field. All of these need to be checked and adjusted accordingly.

In a multi-tenant environment, this restricts the number of users to only one, which is not desired. Single-tenant mode should not be affected by this.

#3 Updated by Eric Faulhaber almost 2 years ago

  • Assignee set to Ovidiu Maxiniuc

#4 Updated by Eric Faulhaber almost 2 years ago

  • Priority changed from Normal to High

#5 Updated by Ovidiu Maxiniuc almost 2 years ago

I have created task branch 9036a and committed the changes to problems I encountered. Latest revision is 15386. Below is a list of patterns of using the Database I identified, with code references, quick analysis and possible solution for fixes, where the case:

The most important usage of Database in FWD is as key in a hashmap. Using this poses some problems. I have collected all these occurrences and investigated locally each of them:
  • ConnectionManager (all occurrences fields in context local ConnectionManager)
    • ldbMap: this seems to be correctly populated (mapping to name of the tenant-default database) but the disconnectImmediately will probably cause incorrect behaviour.
    • pdbReferences: only currently connected tenant databases
    • sessions: maps 'network sessions'. Not sure whether these are related to tenants.
    • ConnectionDispatcher.listeners: used for updating _Connect and _MyConnection meta tables.
  • DatabaseManager (all static)
    • dbConfigs - contains configs for all known databases. Looks OK.
    • schemaByDatabase - map seems OK
    • dbSessions - vaslues seem OK
    • dialects - PROBLEM: was incorrectly populated. Fixed in r15386.
    • transientDatabases - unknown at this time
    • lockTableUpdaters - accesses to object seem to lack synchronization
  • DirtyTempTableHelper.indexCache (static)- unknown at this time
  • FastFindCache
    • permanentInstances (static) - a bit curious, these are not used with the tenants, even if the database could create different caches
    • tenantInstances (static not database mapped) - instead these are used in case of tenant id is present.
  • IndexHelper.cache (static, accessed via IndexHelper.get())
    • ISSUE: based on factory method above, an instance of IndexHelper is cached in RecordBuffer and DefaultDirtyShareManager. However, even if the database changes (as result of tenant change), the IndexHelper is still correct. The issue is that multiple IndexHelper are constructed or same schema. The key here should be the schema name or the default-tenant database.
  • PersistenceFactory.cache (static, accessible via PersistenceFactory.getInstance())
    • ISSUE: based on the factory method above, the persistence is also cached probably incorrectly in RecordBuffer, LockTableUpdater.UpdateWorker, ConnectTableUpdater, LockManager, LockManagerMultiplexerImpl, IdentityManagerMultiplexerImpl, UserTableStatUpdater. These may affect incorrect data (from incorrect database)
  • SchemaCheck.instances (static) no issues identified
  • TxWrapper (all occurrences as fields in context local WorkArea)
    • txWrapperMap and inactiveTxWrappers: possible ISSUE: what happens when the tenant changes during a transaction bock?
    • activeDatabases: ISSUE: since it is initialized and cached from the database property of a RecordBuffer, the map will contain incorrect data if not updated when tenant changes
  • DirtyShareFactory
    • cache (static): ISSUE: not updated when the tenant changes. DirtyShareFactory.getContextInstance() is used with the database parameter at the time the buffer is initialized first time;
    • local (wrapped in a ContextLocal object) - same as above
  • MetadataManager (all static)
    • coreDatabases - this maps tenant databases to their default-tenant database, in a similar way to ConnectionManager.ldbMap, but using Database instead of String.
    • FILE_NUM_BY_TBL_NAME (static) - this seems fine. It is processed via coreDatabases. Because of this, some calls execute correctly even if the tenant database has changed (but the default-database is kept)
    • TABLE_IDS, FILE_NUM (static) - contrary to the above, these use tenant database name. Apparently, they do not corrupt data, but only duplicate it, causing memory waste.
  • UserTableStatUpdater.instances (static) - This collect information from all databases and caches reference to Persistence objects. I am guessing this works fine, though.
  • orm.Settings.all (static) - With the fix in r15386 this seems to work fine.
Database as field member used for driving specific operations:
  • Persistence
    • database: overridden by value in the context, but sometimes overwritten (when?)
      • PROBLEM: he dialect field is kept even if the database is overridden/overwritten
    • the lockTableUpdater and lockManager may lock incorrect items or locks may not be visible when tenant is changed. (Note that the persistence is cached and even if the Database object overridden, this field will do locks in original database;
  • DynamicTablesHelper.parseTableName(): may not return the expected value because the physicalName is used in the process.

Caching of database-specific objects based on the Database value at initialization (or first usage)
Example: RecordBuffer.bufferType, and above.
This will likely to cause problems in tenant enabled mode, regardless if multiple users are connected.

Database as schema identifier
In DatabaseManager, methods getDatabaseDMOs() and getDatabaseTables(), etc: This works on an incorrect assumption that the Database's name is always the same as the schema name (and the path fragment after dmo). This is WRONG! and must be fixed.

Database name exposed to 4GL
BufferFieldImpl.getDbName() will extract the db name. Not sure if this is correct.

Compare Database objects for deciding whether two items belong to same database.
Example: BufferManager.vetoStop(), CompoundQuery$Optimizer.isServerJoinPossible(). If the Database objects are updated consistently and accessed via getters (as opposed to direct field access, which may be overridden) this is OK.

Usage of Database to detect the type of database (like meta, temp, dirty).
Addition usage here is to initialize other objects, like URL for remote connections.
Provided that the Database object are correctly constructed and initialized, this pose no problem. There was a problem in r15385, but it should not be any more.

#6 Updated by Ovidiu Maxiniuc almost 2 years ago

  • % Done changed from 0 to 10
  • Status changed from New to WIP

Multiple issue reported in my previous note are caused because some objects are cached locally, in different persistence objects for fast access. For example, RecordBuffer objects are initialized once with a specific persistence object, based on the database this persistence object is configured with. The indexHelper, persistence, persistenceContext, lockContext, dirtyContext, bufferType, and metadata. If the tenant changes, some of these will keep their value. In some cases this is correct (metadata will remain the same, as well as indexHelper and persistence for the moment, the latter because the object itself will (be) adjust (-ed) to the new tenant database), but some will clearly not (lockContext, dirtyContext will cause interferences with the initial database).

To fix the above issues, the tenant database sensitive fields should cease to exist and extracted directly from the source of truth. In this case, this is the persistence object. Obtaining these values should be done in a similar way the getDatabase() is implemented, by delegating the call to (current) persistence object: if the persistence itself or one of its properties (like the database) changes, the RecordBuffer will get the fresh value.

Important note: I do not think the current implementation of Persistence.getDatabase() is optimal and neither correct. Each time it is called it will access the local context, causing a performance hit, even if the current database is not tenant enabled. Secondly, using the context to store the contextDatabase is not the right way. Assuming there are at least two different databases (for example, one permanent and one _temp), setting the a new tenant database for one (therefore initializing local.contextDatabase) will make all persistence objects to believe they represent the same database (since local.contextDatabase is the same for all objects in the same user context).

The conclusion from the note above is that the Persistence objects must not be 'mutable' from one tenant to another. Instead they should be stable, in a 1-to-1 relation with active databases, (tenant or not). Instead, the RecordBuffer should be coupled with the right Persistence (as bound by the same default-database), at the moment the tenant changes.

I also learnt recently that it is not mandatory that all tables from a database are 'divided'/multiplexed by tenants (#9057, maybe this should be copied/moved there?). The suggested approach for supporting this was to add flags to DMO interface. I think the best solution is a runtime flag (in directory.xml). I see a potential implementation issue: in the case of a query which joins a common table and a tenant table from what looks like the same database, is actually a cross-database operation (even if the two databases share the same schema). Although we have discussed about it before, FWD does not support this currently.

#7 Updated by Greg Shah almost 2 years ago

I think the best solution is a runtime flag (in directory.xml)

Whether or not a table is shared or multi-tenant is known at conversion time (it is part of the schema). It does not change at runtime for different installlations. It is hard coded into the source code, so it should not be a runtime flag.

#8 Updated by Eric Faulhaber almost 2 years ago

To summary (hopefully most or all of) our discussion today...

Database Object Use

We agreed that we want to avoid having mappings (and perhaps reverse mappings) between the tenant-specific Database instances and the "logical" (not in the OE sense, but rather meaning the default tenant) Database instances. The mapping approach would require expensive map lookups to be introduced everywhere, potentially having adverse performance effects on even single-tenant mode.

It seems all the use patterns of the Database class could be addressed with the least amount of code changes by making the Database class "multi-tenant friendly". This would entail adding an instance variable to hold a reference to a database's default tenant Database instance. That reference would be accessed for all cases where the default tenant Database object was needed (e.g., as a map key for data that is common across tenant databases). In single-tenant mode, this reference would be to the enclosing instance itself. A getter method and appropriate c'tor variants (or a setter method) will need to be added to the class to use this reference.

I don't think this new reference should be used in the implementation of the hash code calculation or the overridden equals method.

It will be up to the logic which uses the Database object to know which "flavor" of Database it needs (default tenant or tenant-specific) to do its work, based on the pattern analysis in #9036-5. To the degree possible, we want to make minimal changes to the existing logic which uses these objects.

Persistence Changes

We also discussed the way the Persistence class is used.

There should be one instance of the Persistence class for each "logical" (again, not in the OE sense) database, meaning each default tenant database. That instance's database object (if that instance variable survives the refactoring) will be set early and not change.

Instance variables which must vary by tenant will be refactored into an inner class (Persistence$Tenant?):

  • dmoVersion: Object which keeps track of changes made to DMOs associated with a tenant database
  • lockManager: Manager of all pessimistic locks for a tenant database
  • lockTableUpdater: Object which updates live lock metadata for a tenant database
  • identityManager: Object which allocates new primary key values for a tenant database

In addition, it needs to be determined if the following can vary by tenant AND if we will support this differentiation by tenant, based on current customer requirements:

  • codepage
  • collation

There will be a Tenant instance for each tenant database associated with a given Persistence object. I think this applies to the default tenant as well. When a persistence context switches to a new tenant, the Tenant associated with that tenant will be set as the active Tenant object within the context-local Persistence$Context instance. Any requests to the Persistence object for any of the above objects which are currently Persistence instance variables are delegated to the active context's current Tenant object.

We didn't discuss this on the call today, but we might want to consider grouping the Persistence$Context instance variables which are specific to the active tenant (e.g., nursery, ffCache, etc.) into a separate class as well, to make the management/maintenance of these clearer.

The resulting relationships between Persistence and its related classes will be as follows:

  • One PersistenceFactory instance to many Persistence instances
    • Persistence instances are mapped by default tenant Database keys
  • One Persistence instance to one default tenant Database
    • stored in the database instance variable
  • One Persistence instance to many Persistence$Context instances
    • this remains unchanged
    • Persistence$Context instances are stored in the context-local Persistence.context instance variable
  • One Persistence instance to many Persistence$Tenant instances
    • Persistence$Tenant is not a context-local variable of Persistence
  • One Persistence$Tenant instance to one default tenant Database
  • One Persistence$Context instance to one Persistence$Tenant instance (at a time)
    • to clarify: only one Tenant instance can be active in a Context instance at a time; however, the same Context can be switched to use a different Tenant instance any number of times.

The idea of this refactoring is to encapsulate as much of the multi-tenancy state into the Persistence object, so that code which uses this class can change as little as possible and remain tenant-unaware, to the degree possible.

That being said, the objects being moved into the Persistence$Tenant inner class (or whatever we ultimately call it) and the objects subject to tenant switching in Persistence$Context (e.g., ffCache) may currently be cached in various, tenant-unaware classes around the persistence runtime. These cached values will become stale if there is a tenant-switch. Thus, instead of caching these instances, we will need to retrieve their values as needed from the current persistence context. To avoid a context-local lookup in the Persistence API, we should retrieve these using Persistence$Context instances directly (like we already do with RecordBuffer.persistenceContext). This may require storing instances of Persistence$Context in more places than it is today.

#9 Updated by Ovidiu Maxiniuc almost 2 years ago

9036a branch was updated. Latest revision is 15387.
Changes:
  • Made Database objects tenant-friendly;
  • Started to enforce single Persistence per set of tenant databases policy;
  • Database-related optimisations;
Issues detected:
  • the tenant databases are eagerly connected at server startup, even before the default tenant-database;
  • along with these, the meta and dirty secondary databases are also connected and populated;
  • since I have two tenant database, the initialization is performed twice and since some operations are not that resilient, the server crashes, ATM;
  • the default startup of the client should set the persistence to default tenant database. The db setup for the tenant is not necessary, and should be delayed as much as possible;
  • there is a chance that the meta db connection to be kept when the tenant changes (although do not know how this works with mutable databases?). I think tenant databases cannot be mutable.

#10 Updated by Ovidiu Maxiniuc almost 2 years ago

9036a branch was updated. Latest revision is 15388.
Changes:
  • Improve database awareness of tenant information;
  • Delayed connection to tenant database until actually needed.

#11 Updated by Ovidiu Maxiniuc almost 2 years ago

Earlier today I committed my latest changes to 9036a and rebased the branch to latest trunk. The current revision is 15409.

While working, I encountered the following question: should the physical _meta database be common or each tenant should have its own version of it?
At this moment, the Persistence object responsible for this database is switching the physical database together with the primary database. I need to think whether this is really necessary, because each time this database is connected/activated there is a non-trivial process for populating some tables (more in the future). But the information is pretty much similar (like _File and related tables). There are some exceptions, for example _Connection and _Lock should contain information on the newly set (primary) tenant database. Maybe this is where we need to separate the VST from non VST tables?

#12 Updated by Greg Shah almost 2 years ago

But the information is pretty much similar (like _File and related tables).

These databases share the same exact schema so schema-related metadata should indeed be common.

There are some exceptions, for example _Connection and _Lock should contain information on the newly set (primary) tenant database. Maybe this is where we need to separate the VST from non VST tables?

This is a good point. I do think we should test in a multi-tenant OE environment to see how these tables respond. For example, can you see connections for all tenants or is it isolated to only the connections for the current tenant?

#13 Updated by Ovidiu Maxiniuc almost 2 years ago

The investigations for _meta database lead me to other issues. We discussed today and here is a summary of our meet.

First some terminology (for colleagues new to tenant and for further references below):

  • There are 3 types of tenants:
    1. the default tenant. This is some kind of 'guest' user, which can access common tables, but has some restrictions. There is only one default tenant per multi-tenant database;
    2. regular tenants. After authentication, they can access both shared tables and their private tables;
    3. super-tenant. It does not have its own data but can access all shared tables and all multi-tenant data in the database (of course, not all at once, one at a time. I don't know yet the the mechanism for selecting the active instance).
  • There are 3 types of tables in a tenant-enabled database:
    1. shared tables - these are normal tables whose content is accessible by all tenants, the same way as in a not tenant enabled database;
    2. multi-tenant tables - these are private tables, each tenant can store and fetch data only from its own table instance (I do not know yet whether the PKs can be duplicated in 4GL, but this should not be a problem in FWD). However, these tables are NOT accessible to default tenant. Attempts from the default tenant to access these will cause errors to be raised;
    3. default-enabled multi-tenant tables. As above, each tenant has its own instance of the table, including the default tenant, this time.

With the above being said, a few issues were identified in the initial approach of the second round of the implementation.

1. A Persistence object for a logical database is not enough

My commits in 9036a implemented a kind of multiplexing of Persistence object so that it uses the connection to single physical database at a given moment. When the tenant changes (re-authenticate), the Persistence will switch to the new private database. This solution is not correct.

A tenant-enabled database can define both shared and private tables. To make the data private, the multi-tenant tables will be stored in separate physical databases, specific to each tenant. OTOH, the shared tables/data will be stored in a database commonly accessible to all tenants.

Now consider a procedure which defines two buffers for tables from same database: one is private, one shared. Since both belong to same logical database, they will share the Persistence object. This means that both of them will access the current physical database currently active in Persistence object. This is wrong, because they point to tables from different physical databases. Therefore Persistence objects must be created and maintained for each physical database, like this:

Buffer1 (Shared-Tenant-Table) ---> Persistence1 ---> shared database (physical and logical names identical)

Buffer2 (Private-Tenant-Table) ---> Persistence2 ---> tenant database1 (physical name different from logical name)

2. Switching to multi-tenant database mode requires reconversion

Of course, the current schemas we generate is not multi-tenant aware, so the DMOs will get a new attribute in their @Table annotation to specify which of the above type of table they represent.

But this is not all. Because of the buffers of same logical database are not mandatory in same physical database, some queries will NOT convert the same. This is only the case when a query involves multiple buffers of same logical database. Now FWD conversion tries to generate optimal FQLs for making a join possible on server (SQL) side (PreselectQuery). However, for buffers whose tables are not in same database this will not be possible, so a CompoundQuery will be generated. Therefore the business code may suffer changes. Without the reconversion, the table will not be looked up in correct database or the SQL JOIN statements will fail.

3. Possible performance penalty

In the above case where a join was performed on SQL side in common database but now a cross physical-database query is necessary, the final result of the query must be processed and assembled on FWD side, in the same manner as query is performed between tables from permanent tables and temp-tables. Depending on the complexity of the query, the performance hit might be or not noticeable. All other cases (when the JOINs are performed on tables from same physical database, but all from same shared or private database) will NOT be affected.

4. Metadata support

The first approach was to have metadata stored in different databases, but this pose the problem of multiplicating identical data in memory. Taking into account that there can be hundreds of tenants defined, this would be a waste of resources. Additionally, there is some data which is common, updatable, and therefore should be shared. The solution seems to be to multiplexing, the same way as we do with the other in-memory database, temp-tables. This would avoid unneeded data duplication (we multiplex only data for private tables, not _File, _Field and related). Since we already have the multiplexing support for _temp database, the extension should not be too difficult.

We decided to approach the above issue in multiple stages:

  1. gather additional low-level information of behaviour of tenant-enabled application, including: native 4LG metadata, data export (schema/data), behaviour in edge-cases;
  2. revert some changes in 9036a to enforce 1-1 relation between Persistence objects and physical databases;
  3. add conversion support;
  4. implement multiplexer for specific (investigate which) _meta tables.

Depending on the progress, these will me delivered in subsequent branches (9036b, 9036c).

#14 Updated by Eric Faulhaber almost 2 years ago

Ovidiu Maxiniuc wrote:

1. A Persistence object for a logical database is not enough

My commits in 9036a implemented a kind of multiplexing of Persistence object so that it uses the connection to single physical database at a given moment. When the tenant changes (re-authenticate), the Persistence will switch to the new private database. This solution is not correct.
[...]
We decided to approach the above issue in multiple stages:
[...]
  1. revert some changes in 9036a to enforce 1-1 relation between Persistence objects and physical databases;

We discussed this, but I don't think it is a settled point yet. If we revert the changes you made, it will break the encapsulation of the Persistence class and make all users of this class much more complex.

Why can the Persistence$Tenant object we designed earlier not handle and (mostly) hide the fact that a tenant needs access to two physical databases (one for private data and one for shared data), within the context of the logical database which the Persistence object represents? There will still need to be changes in other parts of the runtime (like SQL query generation for joins), which will need to be aware of this distinction, but I am trying to avoid redesigning more of the persistence framework than is absolutely necessary.

#15 Updated by Eric Faulhaber almost 2 years ago

I've also been thinking more about the "need to reconvert for multi-tenancy mode" aspect we discussed. We noted that joins across shared and private tables across different physical databases was the driver for this. To a secondary degree, tying the table type (shared, multi, multi+default) to the DMO interface within the @Table annotation would require conversion (and re-conversion, if a table type was ever changed) as well.

Constantin asked (at least, I think this is what he meant) whether we could just convert everything to CompoundQuery and sort it out at runtime. I answered that the runtime optimizer within CompoundQuery was not sophisticated enough to refactor CompoundQuery instances back to, say, a PreselectQuery that the "non-tenant" conversion (i.e., the current conversion) would have selected.

However, what if we look at it the other way around: there are only a few query types that support multi-table joins (e.g., PreselectQuery, PresortQuery, ...). What if we left the current conversion alone and made only runtime changes that would check (only in multi-tenant mode) whether we had a "mixed mode" query, which joins shared and private tables. In those cases only, we could refactor the query components of that query into a CompoundQuery and execute that. It is much easier in my opinion to go this direction than to try to refactor a CompoundQuery into a query that does the join on the server side (we can see the complexity of that effort in the CompoundQuery runtime optimizer).

To prevent having to re-convert upon changing the type of table, we could treat the configuration of table-type like any other landlord data. It would be managed via REST and stored in the landlord database, and cached at runtime in the persistence layer for fast access.

#16 Updated by Ovidiu Maxiniuc almost 2 years ago

Eric Faulhaber wrote:

We discussed this, but I don't think it is a settled point yet. If we revert the changes you made, it will break the encapsulation of the Persistence class and make all users of this class much more complex.

I am sorry. That was a big amount of information and I may wrote that incorrect. Please feel free to correct me.
But why do you mean by breaking the encapsulation? Pinning the Persistence object to a specific physical database has as effect that its database field can be final and the dmoVersion, lockManager, lockTableUpdater and identityManager do not need to be aggregated into a container since they do not need to be updated. If a record buffer was initialized as 'attached' to a Persistence for fwd database, when the tenant changes to physical database fwd-ten2, it will be released (within the old (default-) tenant) then 'reattached' to the Persistence of fwd-ten2. Of course, the buffer must not cache the persistence context; or, if it does, it must be updated at the same time with the persistence. In other words, instead of altering the Persistence objects we reconnect the buffers to existing Persistence mapped by the newly set tenant Database. They will load records from same table, but different physical database.

Why can the Persistence$Tenant object we designed earlier not handle and (mostly) hide the fact that a tenant needs access to two physical databases (one for private data and one for shared data), within the context of the logical database which the Persistence object represents? There will still need to be changes in other parts of the runtime (like SQL query generation for joins), which will need to be aware of this distinction, but I am trying to avoid redesigning more of the persistence framework than is absolutely necessary.

I think it is not necessary.
The join optimizer will test the databases and will create the server-side JOIN only if they (the databases) are identical. I estimate this will not be changed.

#17 Updated by Ovidiu Maxiniuc almost 2 years ago

Eric Faulhaber wrote:

I've also been thinking more about the "need to reconvert for multi-tenancy mode" aspect we discussed. We noted that joins across shared and private tables across different physical databases was the driver for this. To a secondary degree, tying the table type (shared, multi, multi+default) to the DMO interface within the @Table annotation would require conversion (and re-conversion, if a table type was ever changed) as well.

One important attribute to be aware of in runtime is the tenant type of the table (one of the types in note-13). Of course, all this information can be set dynamically in landlord database, or even manually set in directory (if feasible, because we have seen databases with 1000+ tables).

Constantin asked (at least, I think this is what he meant) whether we could just convert everything to CompoundQuery and sort it out at runtime. I answered that the runtime optimizer within CompoundQuery was not sophisticated enough to refactor CompoundQuery instances back to, say, a PreselectQuery that the "non-tenant" conversion (i.e., the current conversion) would have selected.
However, what if we look at it the other way around: there are only a few query types that support multi-table joins (e.g., PreselectQuery, PresortQuery, ...). What if we left the current conversion alone and made only runtime changes that would check (only in multi-tenant mode) whether we had a "mixed mode" query, which joins shared and private tables. In those cases only, we could refactor the query components of that query into a CompoundQuery and execute that. It is much easier in my opinion to go this direction than to try to refactor a CompoundQuery into a query that does the join on the server side (we can see the complexity of that effort in the CompoundQuery runtime optimizer).

Yes, I agree to that.

To prevent having to re-convert upon changing the type of table, we could treat the configuration of table-type like any other landlord data. It would be managed via REST and stored in the landlord database, and cached at runtime in the persistence layer for fast access.

I agree to this, too, but in case of large databases the configuration might be an issue (see above in this note).

#18 Updated by Eric Faulhaber almost 2 years ago

Ovidiu Maxiniuc wrote:

But why do you mean by breaking the encapsulation? Pinning the Persistence object to a specific physical database has as effect that its database field can be final and the dmoVersion, lockManager, lockTableUpdater and identityManager do not need to be aggregated into a container since they do not need to be updated.

These fields should not be updated. They should be final, initialized when the Tenant object is first created. There should be one Tenant object per physical tenant database. I was envisioning that all Tenant instances would be in a map, keyed by tenant ID.

The default tenant's Tenant object would be created when the Persistence object is initialized (unless this is too early), and each tenant's Tenant object would be created lazily, upon the first switch to that tenant. All would be cached in a tenant map (an instance variable of Persistence, probably a ConcurrentHashMap).

As currently written in 9036a/15409, Persistence$Tenant is not used as I intended; I'm sorry for not explaining it better. tenant should not be an instance variable of Persistence. Rather, it should be an instance variable of Persistence$Context, representing the active tenant for the current context. The Persistence.defaultTenant instance variable is not strictly necessary, since that Tenant instance should be stored in the map with the reserved key "default". It might be useful as a performance optimization, though, to avoid the lookup, if that happens often enough.

Persistence represents the logical database (i.e., the family of all physical, tenant databases for that logical database, including the default tenant database with all shared tables). If we leave the tenant instance variable where it is now, then every time we switch a tenant in any session, it switches the tenant variable for all contexts using that logical database. This would break with any concurrent use.

If a record buffer was initialized as 'attached' to a Persistence for fwd database, when the tenant changes to physical database fwd-ten2, it will be released (within the old (default-) tenant) then 'reattached' to the Persistence of fwd-ten2. Of course, the buffer must not cache the persistence context; or, if it does, it must be updated at the same time with the persistence. In other words, instead of altering the Persistence objects we reconnect the buffers to existing Persistence mapped by the newly set tenant Database. They will load records from same table, but different physical database.

I want as much awareness about the active tenant to be within the boundary of the Persistence class, so that the logic changes to other persistence classes is minimal. I do not want listeners implemented everywhere to manage tenant switches, registering new Persistence instances whenever there is a switch. A RecordBuffer should only know about one Persistence instance, representing the logical database the buffer thinks it's using. Any such switching should only be happening only within the persistence context. Other classes, to the degree possible, should not even be aware of it.

Like Database, the Persistence class is used all over the persistence framework, and all the logic which uses it currently was written with the assumption that it represents a single database, not a logical family of databases. Rather than changing all those users of the Persistence and Persistence$Context classes, I suspect the simpler and more OO way to support the new meaning is to hide that knowledge inside the Persistence class. If you believe otherwise, please convince me. I am just trying to get to the best long term design with the quickest implementation path.

The idea of "hiding" the shared vs. private table implementation inside Persistence is that any API call into Persistence or Persistence$Context would just be using the Persistence object for the logical database, not having to be aware that there are potentially two Session instances inside to manage access to separate, physical databases, one for private tenant tables and another for shared tables.

I assume the knowledge of the table type would be in the DmoMeta class, potentially initialized at server startup from the landlord database.

Why can the Persistence$Tenant object we designed earlier not handle and (mostly) hide the fact that a tenant needs access to two physical databases (one for private data and one for shared data), within the context of the logical database which the Persistence object represents? There will still need to be changes in other parts of the runtime (like SQL query generation for joins), which will need to be aware of this distinction, but I am trying to avoid redesigning more of the persistence framework than is absolutely necessary.

I think it is not necessary.
The join optimizer will test the databases and will create the server-side JOIN only if they (the databases) are identical. I estimate this will not be changed.

Sorry, what is the join optimizer?

#19 Updated by Greg Shah almost 2 years ago

2. Switching to multi-tenant database mode requires reconversion

Please note that an important objective of our multi-tenant approach is to make the same converted application run in both single-tenant and multi-tenant modes. The table-level annotation can help us with our implementation of shared vs multi-tenant tables but should be ignored when running in single tenant mode.

I like the idea above in #9036-15 that we hide the implementation of the query at runtime.

#20 Updated by Eric Faulhaber almost 2 years ago

Greg Shah wrote:

[...] The table-level annotation can help us with our implementation of shared vs multi-tenant tables but should be ignored when running in single tenant mode.

Do we need the flexibility of setting table type as I proposed in the last sentence of #9036-15? If we use a table annotation for this metadata, then it is fixed at conversion, and requires a re-conversion if changed later. This is ok if the table type is considered part of the schema, perhaps less so if it is something a DBA would normally determine.

#21 Updated by Ovidiu Maxiniuc almost 2 years ago

Eric Faulhaber wrote:

Ovidiu Maxiniuc wrote:

But why do you mean by breaking the encapsulation? Pinning the Persistence object to a specific physical database has as effect that its database field can be final and the dmoVersion, lockManager, lockTableUpdater and identityManager do not need to be aggregated into a container since they do not need to be updated.

These fields should not be updated. They should be final, initialized when the Tenant object is first created. There should be one Tenant object per physical tenant database. I was envisioning that all Tenant instances would be in a map, keyed by tenant ID.

In 9036a the field members of Tenant are not updated. Some of them are final but some of them are not, because they are initialized later.

The default tenant's Tenant object would be created when the Persistence object is initialized (unless this is too early), and each tenant's Tenant object would be created lazily, upon the first switch to that tenant. All would be cached in a tenant map (an instance variable of Persistence, probably a ConcurrentHashMap).

From current revision of 9036a, the map is not used. The reason was the map key. I was not not fully sure about Database equals and hashcode implementation, but after a second look over them the object seems suitable.

As currently written in 9036a/15409, Persistence$Tenant is not used as I intended; I'm sorry for not explaining it better. tenant should not be an instance variable of Persistence. Rather, it should be an instance variable of Persistence$Context, representing the active tenant for the current context. The Persistence.defaultTenant instance variable is not strictly necessary, since that Tenant instance should be stored in the map with the reserved key "default". It might be useful as a performance optimization, though, to avoid the lookup, if that happens often enough.
Persistence represents the logical database (i.e., the family of all physical, tenant databases for that logical database, including the default tenant database with all shared tables). If we leave the tenant instance variable where it is now, then every time we switch a tenant in any session, it switches the tenant variable for all contexts using that logical database. This would break with any concurrent use.

Understood. I will update the code here.

If a record buffer was initialized as 'attached' to a Persistence for fwd database, when the tenant changes to physical database fwd-ten2, it will be released (within the old (default-) tenant) then 'reattached' to the Persistence of fwd-ten2. Of course, the buffer must not cache the persistence context; or, if it does, it must be updated at the same time with the persistence. In other words, instead of altering the Persistence objects we reconnect the buffers to existing Persistence mapped by the newly set tenant Database. They will load records from same table, but different physical database.

I want as much awareness about the active tenant to be within the boundary of the Persistence class, so that the logic changes to other persistence classes is minimal. I do not want listeners implemented everywhere to manage tenant switches, registering new Persistence instances whenever there is a switch. A RecordBuffer should only know about one Persistence instance, representing the logical database the buffer thinks it's using. Any such switching should only be happening only within the persistence context. Other classes, to the degree possible, should not even be aware of it.

Each record buffer should be aware of its physical database. Even such instance is connected to a Persistence object to benefit from it, at a given moment, the buffer must request its operations to be executed for its database. For example, if we have:
  • t1 a common shared table in logical database fwd
  • t2 a private table in same database.
    When we run a procedure which queries these two tables:
  • as default tenant:
    • t1 will use the local/fwd/primary database;
    • t2 is NOT available in the default fwd database, therefore some error will be raised.
      Both buffers at this moment are connected to same Persistence and have the same local/fwd/primary database.
  • when tenant_1 is used
    • t1 will continue use access local/fwd/primary database since there is its physical SQL database located;
    • t2 will use access local/fwd_1/primary database and accessing it will cause an error.
      The buffers still use the same Persistence object and even if it has a new Tenant the current Context, these buffers must access different physical databases. In fact, t1 keeps accessing the 'default' Tenant object, for its entire lifetime.

Like Database, the Persistence class is used all over the persistence framework, and all the logic which uses it currently was written with the assumption that it represents a single database, not a logical family of databases. Rather than changing all those users of the Persistence and Persistence$Context classes, I suspect the simpler and more OO way to support the new meaning is to hide that knowledge inside the Persistence class. If you believe otherwise, please convince me. I am just trying to get to the best long term design with the quickest implementation path.
The idea of "hiding" the shared vs. private table implementation inside Persistence is that any API call into Persistence or Persistence$Context would just be using the Persistence object for the logical database, not having to be aware that there are potentially two Session instances inside to manage access to separate, physical databases, one for private tenant tables and another for shared tables.

As noted above, t1 and t2 are tables from actually two different physical databases. Different Session instances must be used simultaneously in this case. In this case:

t1.persistence == t2.persistence
t1.persistenceContext == t2.persistenceContext
but
t1.getSession() != t2.getSession()
So, getSession() will have a new body in which it will require from the context a specific session, depending on the table tenant type.

I assume the knowledge of the table type would be in the DmoMeta class, potentially initialized at server startup from the landlord database.

This must be implemented. It can be either static information (written at conversion time as an attribute of the DMO) or at runtime (from directory). To be decided. Even in former case, if the DMO have the new attributes, there will not be any change when running in no-tenant or single tenant mode.

The join optimizer will test the databases and will create the server-side JOIN only if they (the databases) are identical. I estimate this will not be changed.

Sorry, what is the join optimizer?

I meant the CompoundQuery$Optimizer which tests whether a server-side SQL SELECT/JOIN can be constructed for the specified components.

#22 Updated by Eric Faulhaber almost 2 years ago

Ovidiu Maxiniuc wrote:

The default tenant's Tenant object would be created when the Persistence object is initialized (unless this is too early), and each tenant's Tenant object would be created lazily, upon the first switch to that tenant. All would be cached in a tenant map (an instance variable of Persistence, probably a ConcurrentHashMap).

From current revision of 9036a, the map is not used. The reason was the map key. I was not not fully sure about Database equals and hashcode implementation, but after a second look over them the object seems suitable.

If the tenant map is an instance variable of Persistence, then Database is not needed as part of the key. The tenant ID differentiates the various Persistence$Tenant map values sufficiently.

#23 Updated by Ovidiu Maxiniuc almost 2 years ago

  • % Done changed from 10 to 20

Actually, I used the database name, in lowercase. That was also to make sure there are no conflicts with the default tenant/database which is stored as "DEFAULT". I do not know whether there is a difference at this moment between the two. This key is used only once, when the tenant in the current context changes and at that moment both keys are available.

The change was committed to 9036a/15410.

#24 Updated by Greg Shah almost 2 years ago

Eric Faulhaber wrote:

Greg Shah wrote:

[...] The table-level annotation can help us with our implementation of shared vs multi-tenant tables but should be ignored when running in single tenant mode.

Do we need the flexibility of setting table type as I proposed in the last sentence of #9036-15? If we use a table annotation for this metadata, then it is fixed at conversion, and requires a re-conversion if changed later. This is ok if the table type is considered part of the schema, perhaps less so if it is something a DBA would normally determine.

In OE, it is part of the schema and I see little reason to change that. It is not a runtime idea. The 4GL code will be written with the tenancy in mind, so it really is something that is static at conversion time.

#25 Updated by Eric Faulhaber almost 2 years ago

Ovidiu Maxiniuc wrote:

Actually, I used the database name, in lowercase. That was also to make sure there are no conflicts with the default tenant/database which is stored as "DEFAULT". I do not know whether there is a difference at this moment between the two. This key is used only once, when the tenant in the current context changes and at that moment both keys are available.

The change was committed to 9036a/15410.

That doesn't make sense to me. Tenant instances should be stored by tenant IDs. The database name doesn't change during tenant switches, the tenant is what changes. The database name is the same for all tenants for a given Persistence instance. The JDBC URL changes, but the core database name should be the same. Am I misunderstanding something?

#26 Updated by Ovidiu Maxiniuc almost 2 years ago

No, you are right. That was just my brain being lazy.
I addressed the issue. Also, I fixed a problem with re-initialization of Persistence object when tenant changes. Indeed, the default tenant is useful: in case no tenants (except the default) are active, it will store the data as before 9036a, therefore no context lookup is necessary. This happens in multiple places in Persistence.
Latest revision of 9036a is 15426.

#27 Updated by Eric Faulhaber almost 2 years ago

The changes in 9036a/15426 look good to me. What is left to implement for this task to be complete (not considering #9057)?

#28 Updated by Ovidiu Maxiniuc almost 2 years ago

As part of my tests of the 9036a/15426 I used two concurrent users which execute basic CRUD operations on the database and changing the tenant. I identified a problem with the lock manager which I initially fixed. However, after fixing that and doing a second tenant switch, I identified a second problem: it seems that the current Peristence$Tenant is not complete: other fields from Peristence$Context need to be moved to Tenant, or otherwise they will leak information from one tenant to another. So not from one context to another, but inside the same context.

This is the inconvenience I am working on right now. Deciding the maximal set of fields which can remain in Context without causing leakage.

I still do not have a clear image on how we will handle the _meta data, for example, in this particular case, the locking part. Related to this, if 4GL stores the locks in same table, there might be no problem with colliding recids since they multiplex the data therefore not allowing records of different to have the same recids. FWD, OTOH, does not have a method to impose different recids. So while the in-memory locks won't be a problem at the tenant change event, the _lock table might not be able to handle locking of such records, therefore the solution might be to rely on independent tables.

After I finish this, and my concurrent testcases work as expected, I intend to do a test on a larger application, even if it is no multi-tenant enabled or concurrent access, this is necessary for detection of regressions I might introduce.

#29 Updated by Eric Faulhaber almost 2 years ago

Go ahead and move fields from Context to Tenant as needed. Keep in mind that all fields in Tenant must be immutable and common across all users of that tenant. Instances of this class are shared across multiple user contexts concurrently; we cannot modify them during tenant switches within any particular context.

On the metadata, think about a solution, but this fix will be done in a separate pass, after we have delivered the core fix for this task.

#30 Updated by Ovidiu Maxiniuc almost 2 years ago

I fixed locking in tenant mode. The fields of Tenant container which must be final are wrapped in a new class which accepts setting the field once, but not subsequent changes.
Latest revision of 9036a is 15427.

Note:
When writing 4GL code and the tenant is changed, this MUST happen outside any transaction. All buffers must be flushed to database and locks released when this happens. Otherwise, this cannot be done any more. Even if the records are released, the locks are not. I guess a similar paradigm is used in 4GL, too.

#31 Updated by Greg Shah almost 2 years ago

When writing 4GL code and the tenant is changed, this MUST happen outside any transaction. All buffers must be flushed to database and locks released when this happens. Otherwise, this cannot be done any more. Even if the records are released, the locks are not. I guess a similar paradigm is used in 4GL, too.

This seems reasonable but it should be proven with a testcase.

We should also update How to Connect a Tenant to a Database with this note.

#32 Updated by Ovidiu Maxiniuc almost 2 years ago

  • % Done changed from 20 to 50
Short update on task status:
  • I managed to get a stable revision of 9036a after the rebase. Committed revision 15436.
  • I have refactor protected Persistence.initializeInstance() to private Persistence.initializeTenant(Tenant t). Now it is clear the intent of initialization only for tenant data, and in particular for default tenant, when calling from Persistence c'tor;
  • The support for _Connect meta table, is pretty OK in FWD. The user name is not updated (that is the tenant) but it looks to me that we are NOT deleting old connection (when users disconnect :( );
  • In 4GL _Lock meta table is visible across tenants. In fact the _Lock table has a field _Lock-Name which seems to reflect the current tenant name. Also it has two fields, _Lock-TenantId and _Lock-DomainId which reflect the respective values.

Note: all work was done using JDK17.

#33 Updated by Ovidiu Maxiniuc almost 2 years ago

First, the good news.
It seems that a tenant change cannot be performed while in a database transaction. In case this happens the following error condition is raised:

Cannot change tenant identity from within a transaction (16402)
My educated guess pointed to something like this will cause problems in FWD, and I really wondered how 4GL handles this. It looks like in the simplest way possible, by prohibiting tenant identity changes while a transaction is active. TODO: add this check in FWD.

_Tenant meta table
This table stores the information on all current tenants. The code for extracting data I used:
FOR EACH _tenant:
    MESSAGE _TenantId
            _Tenant-Name
            _Tenant-ExtId
            '"' + _Tenant-Description + '"'
            _Tenant-Type "|" 
            _Tenant-DataArea-default
            _Tenant-IndexArea-default
            _Tenant-LobArea-default
            _Tenant-Allocation-default
    .
END.
and a sample output:
 0 Default            "Default Tenant" 0 | 0   0   0   None
 1 regular_tenant_1 ? ?                1 | 100 101 102 Immediate
 2 regular_tenant_2 ? ?                1 | 100 101 102 Immediate
-1 super_tenant     ? "Super tenant"   2 | 0   0   0
-2 super_tenant2    ? ?                2 | 0   0   0
There are several things to notice:
  • _TenantId is 0 for the Default tenant, a positive number for normal tenants and a negative number for super tenants;
  • _Tenant-Type is also 0 for the Default tenant, 1 for normal tenants and 2 for super tenants;
  • the mandatory fields in 4GL (_Tenant-DataArea-default, _Tenant-IndexArea-default, _Tenant-LobArea-default, _Tenant-Allocation-default) do not have any meaning in FWD, but must be initialized to some values. Note: in 4GL they have meaning and are not 0 only for regular tenants.

With 9036a/15436, in FWD, I used the data from landlord database to populate this meta table. However, our permanent landlord database will require some adjustments since it does not accommodate all necessary information.

_Connect meta table

From my tests, this table is shared for all tenants connected to same database. For this, I used the following test procedure.

FOR EACH _Connect WHERE _Connect-Type NE ?:
    MESSAGE _Connect-Id
            _Connect-Usr
            _Connect-Type
            _Connect-Name
            _Connect-Device
            _Connect-Time
            _Connect-Pid
            _Connect-Server
    .
END.

The output is:

1 0 BROK Administrator batch Fri Aug  2 08:41:26 2024 19592 ?
2 1 SERV Administrator CON:  Fri Aug  2 08:41:26 2024 11508 ?
3 2 SQSV Administrator CON:  Fri Aug  2 08:41:28 2024 17148 ?
4 3 SERV Administrator CON:  Fri Aug  2 08:44:48 2024  4336 ?
6 5 SELF               CON:  Mon Sep  9 13:26:15 2024 20348 ?
7 6 OEMA DB_Agent      batch Mon Sep  2 19:15:32 2024  9656 ?
8 7 SELF tenant_1@fwd  CON:  Mon Sep  9 13:27:26 2024 10312 ?
9 8 SELF tenant_2@fwd2 CON:  Mon Sep  9 13:27:55 2024 12028 ?
I was connected as Default tenant. The connection of current user is _Connect-Id = 6. The other users connected to same database had _Connect-Id 8 and 9. As you can see, they are the only entities having _Connect-Type = 'SELF'.

_Lock meta table

As with the _Connect meta table I used two additional different procedures to connect with different users (authenticated with different tenants). The first one locked a record in SHARE-LOCK, the second in EXCLUSIVE-LOCK. The procedure which executes the code for data extraction

FOR EACH _Lock:
    MESSAGE _Lock-Id
            _Lock-Usr
            '"' + _Lock-Name + '"'
            _Lock-Type
            _Lock-Table
            _Lock-RecId
            _Lock-Chain
            "'" + _Lock-Flags + "'" 
            _Lock-TransId "*" 
            _Lock-TenantId
            _Lock-DomainId
    .
END.
also acquired an EXCLUSIVE-LOCK on a 3rd table. The output is:
 1 7 "tenant_1@fwd"  REC     3 3584 1114 'S           '   0 * 1 1
 2 5 ""              REC  -340 8590  828 'S   L       ' 297 * 0 0
 3 5 ""              REC  -340 2548  971 'S   L       ' 297 * 0 0
 4 5 ""              REC  -340 1091  751 'S   L       ' 297 * 0 0
 6 5 ""              REC  -340 2543  966 'S   L       ' 297 * 0 0
 7 8 "tenant_2@fwd2" REC     4 8704   51 'X           ' 296 * 2 2
 8 5 ""              REC     3 1536  302 'X           ' 297 * 0 0
12 5 ""              REC  -340 1089  749 'S   L       ' 297 * 0 0
Evidently, the locks are visible across tenants, even if the table was NOT shared (the record from table 4, in X clusive mode by tenant_2@fwd2). The current user (Default tenant) and tenant_1@fwd locked different records from same table (3, multi-tenant, with default access).

#34 Updated by Eric Faulhaber almost 2 years ago

As discussed in recent standups, the current plan is to defer the metadata work and finish the core refactoring for this task to make concurrent, multitenant access functional. The minimal schema conversion changes Ovidiu already made are being backed out and deferred for #9057, to shorten the regression testing cycle by avoiding the need to convert large applications first. Likewise, mixed shared/private table access is deferred to #9057.

Once Ovidiu commits the latest 9036a changes (expected today), I will review ASAP. Assuming that goes well, the test plan for 9036a is as follows:

  • Concurrent multitenancy functionality:
    • Ovidiu to test multi-tenancy with his standalone test cases (this is expected already to be done by the time 9036a is made ready for review).
  • Non-multitenancy regression testing:
    • Ovidiu to run large GUI application unit tests.
    • Lorian to run ChUI regression tests.
    • Constantin to run ETF and Postman tests for large, REST-based application.

Hopefully, we can get this testing done tomorrow (Wed) and get this branch committed to trunk. Ovidiu, the sooner you can commit your changes for review today, the more likely it is we can get any potential review issues resolved quickly and achieve that goal. Thank you all!

#35 Updated by Ovidiu Maxiniuc almost 2 years ago

  • % Done changed from 50 to 100

Eric,
I committed my changes and rebased the branch to latest trunk. Current revision is 15444.
The conversion changes were committed to brand new 9057a branch.

#36 Updated by Eric Faulhaber almost 2 years ago

Code review 9036a/15442-15444:

Nice work, Ovidiu. My findings...

builtin_functions.rules needs a history entry.

DatabaseManager.getTenantName(String db): At line 962, should the check be ... != 1 or ... < 1? The method returns the current user ID. Is that the same as tenant name? There are multiple users per tenant. Is the tenant name not the same for all of them? If this method is not a core requirement for this revision, ignore this until after testing.

Persistence: Why the /*???*/ comment or commented access modifiers in the declarations of various methods?

Persistence$Context.getSession:

The following is not atomic and it updates a shared data structure:

                        tenant = tenantData.get(tenantId.toLowerCase());
                        if (tenant == null)
                        {
                           tenant = new Tenant(mtDb, tenantId);
                           tenantData.put(tenantId.toLowerCase(), tenant);
                           initializeTenant(tenant); // re-initialize database to fit the new tenant
                        }

I'm not an expert in the ConcurrentHashMap implementation, but I think ConcurrentHashMap.computeIfAbsent with a mapping functions is atomic.

The comment at line 5332 doesn't match the logic.

InMemoryLockManager: release and lockImpl have some new debug output going to System.out. Did you mean to remove that? Same for MetadataManager.populateDatabase.

One final question: did you analyze how Persistence is used for temp-tables, to make sure none of these changes are likely to regress this use case?

#37 Updated by Eric Faulhaber almost 2 years ago

  • Status changed from WIP to Review

Ovidiu, please post here again when the review is addressed and set this to "Internal Test" status. My points are pretty minor, so I don't really need to review again except in the unlikely event you make additional, major changes.

When Ovidiu changes the task status, everyone please execute their parts of the test plan in #9036-34 and report the results as soon as possible. Thank you!

#38 Updated by Constantin Asofiei almost 2 years ago

Ovidiu, 9036a does not compile with Java 8:

     [exec] [ant:javac] p2j/src/com/goldencode/p2j/persist/Persistence.java:802: error: cannot infer type arguments for ContextLocal<T>
     [exec] [ant:javac]    private final ContextLocal<Context> context = new ContextLocal<>()
     [exec] [ant:javac]                                                                  ^
     [exec] [ant:javac]   reason: cannot use '<>' with anonymous inner classes
     [exec] [ant:javac]   where T is a type-variable:
     [exec] [ant:javac]     T extends Object declared in class ContextLocal
     [exec] [ant:javac] p2j/src/com/goldencode/p2j/persist/lock/InMemoryLockManager.java:286: error: cannot infer type arguments for ContextLocal<T>
     [exec] [ant:javac]    private final ContextLocal<Context<T>> context = new ContextLocal<>()
     [exec] [ant:javac]                                                                     ^
     [exec] [ant:javac]   reason: cannot use '<>' with anonymous inner classes
     [exec] [ant:javac]   where T is a type-variable:
     [exec] [ant:javac]     T extends Object declared in class ContextLocal

#39 Updated by Constantin Asofiei almost 2 years ago

Server startup fails with this:

Caused by: java.lang.NullPointerException
        at com.goldencode.p2j.persist.meta.MetadataManager.populateTenant(MetadataManager.java:1386)
        at com.goldencode.p2j.persist.meta.MetadataManager.access$1100(MetadataManager.java:152)
        at com.goldencode.p2j.persist.meta.MetadataManager$SystemTable.lambda$static$4(MetadataManager.java:2236)
        at com.goldencode.p2j.persist.meta.MetadataManager$SystemTable.lambda$new$9(MetadataManager.java:2429)
        at com.goldencode.p2j.persist.meta.MetadataManager$SystemTable.lambda$populateAll$11(MetadataManager.java:2503)
        at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)

because in ArrayList<TenantManager.Tenant> tenants = TenantManager.listTenants(); listTenants returns null, as TenantManager has not been initialized.

Please run Hotel GUI in non-tenant mode and after that works OK, I'll do the ETF tests.

#40 Updated by Ovidiu Maxiniuc almost 2 years ago

  • Status changed from Review to Internal Test

Eric Faulhaber wrote:

Code review 9036a/15442-15444:
builtin_functions.rules needs a history entry.

I had that committed to 9057a. I added the history to 9036a, too.

DatabaseManager.getTenantName(String db): At line 962, should the check be ... != 1 or ... < 1? The method returns the current user ID. Is that the same as tenant name? There are multiple users per tenant. Is the tenant name not the same for all of them? If this method is not a core requirement for this revision, ignore this until after testing.

I think that is correct. The condition is evaluated only in case that db == null (that is, no database parameter was provided). The 4GL method will check if there is only one database connected and return the result for it. The error is raised either when there is no database or at least two of them.

Persistence: Why the /*???*/ comment or commented access modifiers in the declarations of various methods?

I highlighted these as they lowers the access of these method to public. We do not want that, but the methods are now declared in the UnsafePersistence interface which Persistence implements. So they must be public as well.

Persistence$Context.getSession:
The following is not atomic and it updates a shared data structure:
[...]
I'm not an expert in the ConcurrentHashMap implementation, but I think ConcurrentHashMap.computeIfAbsent with a mapping functions is atomic.

I have switched to computeIfAbsent(). I looked at the code, But I did not see any mechanism to make it really atomic, just to allow concurrent retrievals while doing updates, without throwing oncurrentModificationException. The code is cleaner.

The comment at line 5332 doesn't match the logic.

The meaning of it is that the tenant field remains unmodified (the change tenant operation ended as a NOP). The code changed a bit, but I kept the comment.

InMemoryLockManager: release and lockImpl have some new debug output going to System.out. Did you mean to remove that? Same for MetadataManager.populateDatabase.

Thanks for noticing that. Will be removed.

One final question: did you analyze how Persistence is used for temp-tables, to make sure none of these changes are likely to regress this use case?

In case of temp-tables, there is only one tenant, the default one, which all calls will use. I did not add specific code for that because it matches the usage of permanent databases with no additional tenant set (tenantData size is 1).

I committed my changes in revision 15445.
I smoke tested the hotel project with JDK 1.8 and it was OK.

#41 Updated by Eric Faulhaber almost 2 years ago

All: please proceed with the test plan.

#42 Updated by Ovidiu Maxiniuc almost 2 years ago

My unittests of one of the larger customers finished with success. I used the prebuilt binaries. In fact, 9036a/r15446 was even better (one test) than trunk/r15432 I used for baseline.

#43 Updated by Constantin Asofiei almost 2 years ago

ETF testing passed, the other REST app also passed.

#44 Updated by Eric Faulhaber almost 2 years ago

Ovidiu, please notify us when the lock release defect we discussed earlier is resolved. We will need to re-run the regression test plan ASAP. If you have time, can you please try to set up the ChUI regression testing with that revision? When do you expect to have the fix ready?

#45 Updated by Ovidiu Maxiniuc almost 2 years ago

Eric,
Actually, there was no lock issue. FWD behaved like 4GL. The problem was the test conditions in AWS. I executed the tests from procedure editor. For some reasons, even if the executed procedure ends, the editors still keeps the tenant as active. In FWD, reconnecting with the client for running a procedure will start fresh.

I think I mentioned earlier about the fact that the tenant cannot be changed while a transaction is active. That was the actual problem. Since in 4GL the tenant was not changed, the error was not issued. OTOH, in FWD the check was not performed, causing the internals of FWD (including the locks) to behave incorrectly. Technically, a new database was added dynamically to the TxWrapper so, at the end of the transaction, it was unable to release the locks for this database.

I added the transaction check at the set of the tenant and the execution is stopped, without reaching the lock issue. It was a red herring.

I committed the change to r15447. I've redone the unittests and they were a match with the baseline. I do not think the other tests require to be redone since there is only a new validation when the tenant is set (and they are not tenant-enabled).

#46 Updated by Eric Faulhaber almost 2 years ago

Ovidiu, please smoke test the GUI app for which you ran unit testing. If you can log in, open a few screens, and there are no obvious problems, we will queue this for merge.

#47 Updated by Ovidiu Maxiniuc almost 2 years ago

I will. But first, I need to import the full database because the unit-test is performed with a reduced instance. I will notify in this task when it's done (ETA: 1h).

#48 Updated by Ovidiu Maxiniuc almost 2 years ago

The smoke test went fine. I guess we are ready to merge. While the database was imported, I already rebased the branch to latest trunk. Current revision is 15455.

#49 Updated by Eric Faulhaber almost 2 years ago

Please merge 9036a to trunk now.

#50 Updated by Eric Faulhaber almost 2 years ago

  • Status changed from Internal Test to Merge Pending

#51 Updated by Ovidiu Maxiniuc almost 2 years ago

  • Status changed from Merge Pending to Test

Branch 9036a was committed to trunk as r15440 then it was archived.

#52 Updated by Ovidiu Maxiniuc almost 2 years ago

After Constantin picked 9036a changes from the latest trunk, he identified a problem with one of the Tenant fields, namely lockContext. Although the Tenant is stored in the Persistence's context, the container is actually shared between the customers, so it is not actually context bound.

What bothers me is that the problem did not surface in internal tests. To catch it, the application requires to have multiple users concurrently accessing the same table, with heavy locks. Our tests are not expressly designed that way. The customer application I ran the unit-tests have some appservers running in the background at startup, but probably they use specific /different tables, so the problem was not caught.

The solution is to enforce the lockContext to be local:

--- old/src/com/goldencode/p2j/persist/Persistence.java 2024-09-13 16:48:05 +0000
+++ new/src/com/goldencode/p2j/persist/Persistence.java 2024-09-13 18:47:19 +0000
@@ -933,7 +933,7 @@
      private final FutureFinal<IdentityManager> identityManager = new FutureFinal<>();

      /** Context-local record lock context */
-      private final FutureFinal<RecordLockContext> lockContext = new FutureFinal<>();
+      private final ContextLocal<RecordLockContext> lockContext = new ContextLocal<>();

      /**
       * Constructs a {@code Tenant} starting with its name and the database it serves. Not all 'final' fields
@@ -4713,12 +4713,13 @@
       */
      RecordLockContext getRecordLockContext()
      {
-         if (!tenant.lockContext.wasSet())
+         RecordLockContext rlc = tenant.lockContext.get();  
+         if (rlc == null)
         {
-            tenant.lockContext.set(RecordLockContext.get(Persistence.this, bufferManager));
+            tenant.lockContext.set(rlc = RecordLockContext.get(Persistence.this, bufferManager));
         }

-         return tenant.lockContext.get();
+         return rlc;
      }

      /** 
The solution adds a new level of indirection, but it is necessary.

At this moment we have:
  • a shared Persistence object for all physical databases of a logical db connection
  • each of the user has a context in Persistence
  • on each Persistence context, the active Tenant is redirecting the operations to current tenant database, which are shared
  • the tenant.lockContext is specific to a database/tenant, but also bound to a current context. Accessing it require a context lookup from the shared tenant.

I am preparing a subsequent branch with the above change and will start testing immediately. Constantin already confirmed that he used this to fix the issue in the project where the problem was identified.

#53 Updated by Eugenie Lyzenko almost 2 years ago

I also have the issue with trunk 15440 and a large GUI app:

...
24/09/14 02:29:19.334+0300 |  SEVERE | com.goldencode.p2j.util.TransactionManager | ThreadName:Conversation [00000009:UVType1Process], Session:00000009, ThreadId:00000006, User:UVType1Process | Abnormal end; original error:
java.lang.RuntimeException: Value was already set.
    at com.goldencode.p2j.persist.Persistence$FutureFinal.set(Persistence.java:881)
    at com.goldencode.p2j.persist.Persistence$Context.getRecordLockContext(Persistence.java:4718)
    at com.goldencode.p2j.persist.Persistence.load(Persistence.java:2081)
    at com.goldencode.p2j.persist.RandomAccessQuery.executeImpl(RandomAccessQuery.java:5373)
    at com.goldencode.p2j.persist.orm.P2JQueryExecutor.executeImpl(P2JQueryExecutor.java:431)
    at com.goldencode.p2j.persist.orm.P2JQueryExecutor.execute(P2JQueryExecutor.java:332)
    at com.goldencode.p2j.persist.orm.P2JQueryExecutor.execute(P2JQueryExecutor.java:198)
    at com.goldencode.p2j.persist.RandomAccessQuery.execute(RandomAccessQuery.java:4368)
    at com.goldencode.p2j.persist.RandomAccessQuery.unique(RandomAccessQuery.java:2907)
    at com.goldencode.p2j.persist.FindQuery.unique(FindQuery.java:1198)
    at com.goldencode.p2j.persist.RandomAccessQuery.unique(RandomAccessQuery.java:2777)
    at com.goldencode.p2j.persist.FindQuery.unique(FindQuery.java:1118)
    at com.goldencode.p2j.persist.AbstractQuery.silentUnique(AbstractQuery.java:4021)
    at <customer_stuff>.src.sys.apps.Pgetmtcinfo.lambda$getVersionNumber$1(Pgetmtcinfo.java:59)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.functionBlock(BlockManager.java:9943)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1109)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1081)
    at <customer_stuff>.src.sys.apps.Pgetmtcinfo.getVersionNumber(Pgetmtcinfo.java:56)
    at <customer_stuff>.src.sys.apps.PgetmtcinfoMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9672)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9628)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7525)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7540)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4522)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7103)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7006)
    at com.goldencode.p2j.util.ControlFlowOps.invokeFunctionImpl(ControlFlowOps.java:6936)
    at com.goldencode.p2j.util.ControlFlowOps.invokeDynamicFunctionInWithMode(ControlFlowOps.java:3455)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1188)
    at com.goldencode.p2j.util.InvokeConfig.execute(InvokeConfig.java:439)
    at <customer_stuff>.src.sys.apps.Asmtcevent.lambda$constructor$1(Asmtcevent.java:156)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:829)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:806)
    at <customer_stuff>.src.sys.apps.Asmtcevent.constructor(Asmtcevent.java:153)
    at <customer_stuff>.src.sys.apps.AsmtceventMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9672)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9628)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7525)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7540)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4522)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7103)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7006)
    at com.goldencode.p2j.util.ControlFlowOps.invokeInImpl(ControlFlowOps.java:6962)
    at com.goldencode.p2j.util.ControlFlowOps.invokeInWithMode(ControlFlowOps.java:1924)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1350)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:426)
    at <customer_stuff>.src.sys.Ppm.lambda$null$9(Ppm.java:249)
    at com.goldencode.p2j.util.ErrorManager.silentWorker(ErrorManager.java:4292)
    at com.goldencode.p2j.util.ErrorManager.silent(ErrorManager.java:711)
    at <customer_stuff>.src.sys.Ppm.lambda$ppmGetMain$16(Ppm.java:249)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.functionBlock(BlockManager.java:9943)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1109)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1081)
    at <customer_stuff>.src.sys.Ppm.ppmGetMain(Ppm.java:207)
    at <customer_stuff>.src.sys.Ppm.lambda$ppmGetPersistent$18(Ppm.java:308)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.functionBlock(BlockManager.java:9943)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1109)
    at com.goldencode.p2j.util.BlockManager.function(BlockManager.java:1081)
    at <customer_stuff>.src.sys.Ppm.ppmGetPersistent(Ppm.java:306)
    at <customer_stuff>.src.sys.PpmMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9672)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9628)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7525)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7540)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4522)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7103)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7006)
    at com.goldencode.p2j.util.ControlFlowOps.invokeFunctionImpl(ControlFlowOps.java:6936)
    at com.goldencode.p2j.util.ControlFlowOps.invokeDynamicFunctionWithMode(ControlFlowOps.java:3599)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1146)
    at com.goldencode.p2j.util.InvokeConfig.execute(InvokeConfig.java:452)
    at <customer_stuff>.src.sys.Mtcevent.lambda$constructor$4(Mtcevent.java:163)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:829)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:806)
    at <customer_stuff>.src.sys.Mtcevent.constructor(Mtcevent.java:139)
    at <customer_stuff>.src.sys.MtceventMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9672)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9628)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7525)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7540)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4522)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7103)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7006)
    at com.goldencode.p2j.util.ControlFlowOps.invokeInImpl(ControlFlowOps.java:6962)
    at com.goldencode.p2j.util.ControlFlowOps.invokeInWithMode(ControlFlowOps.java:1924)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1350)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:426)
    at <customer_stuff>.src.sys.Utils.lambda$null$0(Utils.java:572)
    at com.goldencode.p2j.util.ErrorManager.silentWorker(ErrorManager.java:4292)
    at com.goldencode.p2j.util.ErrorManager.silent(ErrorManager.java:711)
    at <customer_stuff>.src.sys.Utils.lambda$execute$7(Utils.java:572)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:669)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:642)
    at <customer_stuff>.src.sys.Utils.execute(Utils.java:534)
    at <customer_stuff>.src.sys.UtilsMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9672)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9628)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6543)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6401)
    at com.goldencode.p2j.util.ControlFlowOps.invokePersistentImpl(ControlFlowOps.java:7816)
    at com.goldencode.p2j.util.ControlFlowOps.invokePersistentImpl(ControlFlowOps.java:7634)
    at com.goldencode.p2j.util.ControlFlowOps.invokePersistentSetWithMode(ControlFlowOps.java:2385)
    at com.goldencode.p2j.util.ControlFlowOps.invokePersistentSetWithMode(ControlFlowOps.java:2361)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1305)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:426)
    at <customer_stuff>.src.root.Uitgesteldeverwerking.lambda$execute$6(Uitgesteldeverwerking.java:449)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:669)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:642)
    at <customer_stuff>.src.root.Uitgesteldeverwerking.execute(Uitgesteldeverwerking.java:354)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.goldencode.p2j.util.Utils.invoke(Utils.java:1801)
    at com.goldencode.p2j.main.StandardServer$MainInvoker.execute(StandardServer.java:2896)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2170)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2092)
    at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:696)
    at com.goldencode.p2j.main.StandardServerMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:422)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:235)
    at java.lang.Thread.run(Thread.java:750)
...

And some appservers are now failing to be started. Will use trunk 15439 for Replica today.

#54 Updated by Ovidiu Maxiniuc almost 2 years ago

Eugenie Lyzenko wrote:

I also have the issue with trunk 15440:
And some appservers are now failing to be started. Will use trunk 15439 today.

Thank you for reporting this. I did the test with this project but the issue did not manifest. I did not see appservers complaining. I usually look in the server's log to be sure the spawner is correctly installed (I need to do this every time I switch the active project I am working on) and I can start connecting the clients. Might be also a synchronization issue.

At any rate, I created task branch 9036b and committed the fix. The current revision is 15441. It also passed the unit-tests.

Could you test this branch? Assuming it will work OK for you, too, you may want use it instead of trunk/15439, if possible.

#55 Updated by Eugenie Lyzenko almost 2 years ago

Ovidiu Maxiniuc wrote:

Eugenie Lyzenko wrote:

I also have the issue with trunk 15440:
And some appservers are now failing to be started. Will use trunk 15439 today.

Thank you for reporting this. I did the test with this project but the issue did not manifest. I did not see appservers complaining. I usually look in the server's log to be sure the spawner is correctly installed (I need to do this every time I switch the active project I am working on) and I can start connecting the clients. Might be also a synchronization issue.

At any rate, I created task branch 9036b and committed the fix. The current revision is 15441. It also passed the unit-tests.

Could you test this branch?

Is this a single file to update? Can you attach it here to simplify testing?

Assuming it will work OK for you, too, you may want use it instead of trunk/15439, if possible.

I would prefer to wait is to be in trunk if no objections.

#56 Updated by Ovidiu Maxiniuc almost 2 years ago

Eugenie, please find the changed file attached.

#57 Updated by Eugenie Lyzenko almost 2 years ago

Ovidiu Maxiniuc wrote:

Eugenie, please find the changed file attached.

Thanks, I'll test now and be back with results soon.

#58 Updated by Eugenie Lyzenko almost 2 years ago

The fix works fine, no more errors and appservers are working too.

#59 Updated by Eric Faulhaber almost 2 years ago

  • Status changed from Test to Merge Pending

Please merge 9036b to trunk now.

#60 Updated by Ovidiu Maxiniuc almost 2 years ago

  • Status changed from Merge Pending to Test

Branch 9036b was committed to trunk as r15442 then it was archived.

#61 Updated by Tomasz Domin almost 2 years ago

Ovidiu
There is a regression when accessing remote databases.
Please see #6667-1160

#62 Updated by Greg Shah over 1 year ago

  • Status changed from Test to Closed

The customer confirmed that this is resolved.

#63 Updated by Greg Shah 5 months ago

  • topics Multi Tenant added

Also available in: Atom PDF