Project

General

Profile

Feature #4369

implement stateless FWD server clustering

Added by Greg Shah over 6 years ago. Updated about 2 months ago.

Status:
Test
Priority:
Normal
Assignee:
Eduard Soltan
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Alexandru Lungu, Eric Faulhaber, Greg Shah
production:
No
env_name:
topics:

lua_script_lockManager.patch Magnifier (22.5 KB) Eduard Soltan, 05/18/2026 02:12 AM

Related issues

Related to Database - Feature #1879: push lock manager back into database New
Related to Runtime Infrastructure - Feature #5170: add support for cloud-based load balancing and WAF New
Related to User Interface - Feature #8973: distribute stateful interactive sessions across a cluster of FWD servers New
Related to Runtime Infrastructure - Bug #11157: Improve FWD server startup mode in server.sh Internal Test
Related to Deployment - Bug #11329: Stateless FWD server clustering setup questions New
Related to Database - Bug #11379: implement "live" metadata tables in persistent database New

History

#1 Updated by Greg Shah over 6 years ago

The core idea here is that if customers have written their application to be stateless in the 4GL, then today they can implement multiple appservers/PASOE instances and take advantage of horizontal scaling of the appserver tier while SHARING access to one or more DATABASE instances. We have work to do in enabling to this approach.

  • Customers wanting to move to this mode will need to ensure their application is truly stateless. If not, then it will not be able to work in a clustered environment. This limitation would also exist in the 4GL.
  • All FWD servers would be required to use the same converted application jar(s).

#3 Updated by Greg Shah over 6 years ago

  • Related to Feature #1879: push lock manager back into database added

#4 Updated by Greg Shah over 6 years ago

After a discussion with Eric, we have this list of items that are persistence related.

  • dirty share database
    • This is a quirk of the 4GL implementation where uncommitted index updates in one session are visible to other sessions. It has multiple implications that can be seen from 4GL code.
    • queries in one session can traverse uncommitted records from another session
      • Our idea at this time is that we WILL NOT support this in stateless clustering mode.
      • This is part of the changes that would be needed to make an application stateless.
    • unique constraint violation that is detected early in the 4GL because the index changes are visible across sessions
      • This part needs support.
      • For performance reasons, this is currently being rewritten to use maps.
  • persistence global event queue
    • Used to notify other sessions when there are index changes.
    • This is a different aspect to the same "uncommitted index updates" problem but it also can manifest in cases where there are committed changes.
    • It is used to shift record-oriented loops (e.g. FOR EACH) from adaptive into dynamic mode.
    • This will be needed.
  • identity pool
    • We no longer use recycled primary keys but we do have a small batch of pre-fetched valid IDs which are specific to the server.
    • This was done for performance instead of querying the sequence for each ID.
    • This has to be made safe.
  • locking
    • The current locking approach is all in-memory.
    • We must rework lock manager, _lock metadata and the lock built-in functions.
    • In #1879 we have a potential path to moving this back into the database, which would naturally resolve this issue.
    • Another approach would be to implement a clustered version of the lock manager.
  • metadata usage that is done in-memory using H2
    • _connect
    • edits to _file, _field (reads would naturally be server independent but updates must be shared)
    • check each metadata table to see which ones have editable state which must be shared
  • caches
    • Several persistence data structures/classes/instances are cached for performance or memory reasons today.
    • Some of these might be safe for leaving as a per-server cache with each server having a separate (unsync'd) cache.
    • Each of the caches (dynamic temp-table definitions, queries...) will need to be inspected to determine the right approach.

#5 Updated by Greg Shah over 6 years ago

For data structures which cannot be backed by the database AND which must be shared across JVM instances in the FWD cluster, we will need to implement a facility that handles this synchronization of state in real-time. A possible solution is Hazelcast which is an Apache 2.0 licensed in-memory data grid technology designed for exactly this use-case.

#6 Updated by Greg Shah over 6 years ago

One thing to consider is that we don't want these server instances to be hard to configure. At one time we considered a kind of directory master/directory slave approach where each server in a cluster has a read-only copy of a common multi-server directory and one server was the master where changes could be made.

We still may want to go this way. The actual changes per-server are minimal (cert/private key, IP addresses). Most of the directory can and should be shared/common.

#7 Updated by Greg Shah over 6 years ago

At this time I am assuming the Admin Console is not changing, but we must at least make it work with the directory approach. In a perfect world, the Admin Console would be able to manage the entire cluster. This may not happen in our first pass.

#8 Updated by Greg Shah over 6 years ago

Constantin/Ovidiu: Can you think of any other persistence or even non-persistence state which we need to share across JVMs? Also, please share any other thoughts, comments or questions.

#9 Updated by Eric Faulhaber over 6 years ago

We also should consider the potential duplication of dynamic proxies and other dynamically generated interfaces and classes across server instances. This is touched upon with the mention of caches above and it may be enough to just look at this as a caching issue. However, our use of custom classloaders and dynamically generated classes may also complicate the clustering design. ATM, I can't specifically put my finger on a problem here, but I suspect we will hit something related to this...

#10 Updated by Constantin Asofiei over 6 years ago

Most (if not all) of the non-persistence runtime is context-local. One part I see that may require improvements (if we decide so) is the global publish/subscribe, to work across all servers in the cluster. A review would be needed, but otherwise I don't recall non-persistence state being shared across contexts.

Eric, you mention dynamic proxies and other dynamically-generated Java bytecode - why do you think this matters? Do you think these kind of instances will be used for sending state to other servers in the cluster (like for _connect or other meta tables)?

An alternative for the meta tables would be to store it in a physical database, to which all servers in the cluster connect (maybe the same database as the app's db?). Can a user explicitly lock a record in a meta table, to edit it? If so, then it might make sense to treat these tables the same way as the user-defined tables.

And another note: I think the in-server appserver agents would be a good addition to this (I know we plan to add this).

#11 Updated by Ovidiu Maxiniuc over 6 years ago

First, pleat let me know if I got this tight.
For me, running in a stateless mode means that the clients connect and execute independent (quick) request on one of the available server instances. This means some kind of authentication is performed at each connect. Of course, the 4GL programmer can implement a stateful application using database to store and eventual context there, but the idea is that at the next request the context is available on any of the configured servers.

How about the persistent procedures?
As I see this, they are allowed as long they are unloaded by the end of the request. Leaving them alive will break the state constraint. But, after a deeper though, they should be unloaded as when the client request ends, its context (where the persistent procedures are held) are discarded.

Can the clients execute parallel requests on different servers?

In the end, I see a rather blurred line between current FWD and the stateless support. So maybe I should better think like of what are we missing from supporting running parallel applications correctly on same backing database?

#12 Updated by Ovidiu Maxiniuc over 6 years ago

Constantin Asofiei wrote:

An alternative for the meta tables would be to store it in a physical database, to which all servers in the cluster connect (maybe the same database as the app's db?). Can a user explicitly lock a record in a meta table, to edit it? If so, then it might make sense to treat these tables the same way as the user-defined tables.

Yes, this is the right way to do it, I guess. The metadata are of two kind:
  • persistent. They are user-editable. This is the case of _user table. To create a new account you can write a record directly to this table. This is why we have already decided to keep this particular meta-table with the rest of the database and allow editing and locking on it;
  • VST (Virtual System Tables). They are readable, but not writeable in 4GL. They (usually) contain statistical information on the system/database and which are updated automatically, event-based. Probably they are reset with the 4GL/database server. Since the data should be accessible on all FWD servers it makes sense to store these tables on same database. However, because of the volatile content, they might be implemented as global temporary tables.

#13 Updated by Constantin Asofiei over 6 years ago

Ovidiu Maxiniuc wrote:

How about the persistent procedures?
As I see this, they are allowed as long they are unloaded by the end of the request. Leaving them alive will break the state constraint. But, after a deeper though, they should be unloaded as when the client request ends, its context (where the persistent procedures are held) are discarded.

The business logic can decide to cache some persistent procedures, which is OK, as long as there is no request-specific data there. So, a request can be something like this:
  • the user accesses a login procedure, which checks some credentials and returns a token
  • on each subsequent requests, the token is passed back, so the appserver knows which app user is executing
  • on a certain request, the appserver can decide to i.e. initialize some classes or other kind of persistent programs, and cache these in global vars or other mechanisms. Again, this is OK, as long as the app doesn't cache request-specific state.

Also, this cache is dependent on the appserver mode: for example, in STATELESS, the agent doesn't have its context reset, so any global-shared temp-tables or vars, or OO static fields, will remain there.

#14 Updated by Greg Shah over 6 years ago

I think the in-server appserver agents would be a good addition to this (I know we plan to add this).

I think so too. You'll see a task for that shortly. :)

#15 Updated by Greg Shah over 6 years ago

For me, running in a stateless mode means that the clients connect and execute independent (quick) request on one of the available server instances. This means some kind of authentication is performed at each connect. Of course, the 4GL programmer can implement a stateful application using database to store and eventual context there, but the idea is that at the next request the context is available on any of the configured servers.

Correct.

Can the clients execute parallel requests on different servers?

Yes.

In the end, I see a rather blurred line between current FWD and the stateless support. So maybe I should better think like of what are we missing from supporting running parallel applications correctly on same backing database?

Yes, this is a reasonable way to think about it.

#16 Updated by Greg Shah over 5 years ago

  • Related to Feature #5170: add support for cloud-based load balancing and WAF added

#17 Updated by Greg Shah almost 5 years ago

Another open source framework to consider is MicroStream. It can be used to replicate object graphs across multiple JVMs.

#19 Updated by Greg Shah about 4 years ago

Evidently REDIS also has an object synchronization capability. We should evaluate this in addition to the other frameworks. Please note that one big advantage of REDIS is that Amazon provides a managed REDIS service so there may be some operational benefits there whereas Hazlecast would have to be application managed.

#20 Updated by Greg Shah about 2 years ago

  • Related to Feature #8973: distribute stateful interactive sessions across a cluster of FWD servers added

#22 Updated by Artur Școlnic about 1 year ago

I am a little late to the party, but glancing over this task, I think Docker fits quite well for Stateless FWD Clustering. The containers can be scaled (and managed) easily using Docker swarm or kubernetes, they can use the same volume (shared disk, if needed), can connect to the same database, a load balancer can be employed to manage the load on the containers, they are easy to configure and light weight at runtime. What they cannot do is share caches and in memory lock management. For caches, usually a separate layer (container) is used, this way the caches are easily scalable and can be shared.

#23 Updated by Greg Shah about 1 year ago

We already run in Docker quite nicely. The issue is that there is quite a bit of 4GL compatibility (like database locking) that is implemented in a stateful way in the FWD application server. That state must be synchronized across all FWD servers that would make up a cluster. Without doing that (using something like Hazlecast) we cannot have multiple FWD servers share the load for the same set of calling programs.

#24 Updated by Eric Faulhaber 10 months ago

  • Assignee set to Andrei Plugaru

#26 Updated by Eric Faulhaber 10 months ago

Fully implementing #8388 will eliminate the requirement to cluster dirty share state across servers.

#27 Updated by Andrei Plugaru 10 months ago

Some thoughts about my understanding of this task:
  • The primary work would be on identifying the list of items that should be synchronized between the server instances and implementing this. I see that an initial list has been made in #4369-4. However, some things probably changed since then. Also, I think that this list can be split in two. Firstly, we should identify the data structures that must be synchronized in order to ensure complete functionality. Then, I would assume there will be instances where synchronization is optional, like FF cache(?). For these kind of items, the decision should be mainly based on performance:
    1. Is it faster to keep the cache only at each server level(this implies that on the other server instances the entry in FFcache will not be found); however, this way, the communication overhead to the 3rd party app(e.g. Redis) will be avoided
    2. Take the communication overhead penalty, however the entries in the cache will be visible across all the FWD server instances

Also regarding the synchronization between server instances, the current access to the data structures should still be preserved. We don't want to force anyone to use a 3rd party app for synchronization if they still have a single FWD server instance. This means that the access to the structures should be behind an interface in order to easily switch between single server mode and multi server mode.

  • Regarding the 3rd party service used for sync. As I can see, Redis has been the last one mentioned here. Greg/Eric, please help me understand if a final decision has been taken in this regard. However, no matter what service we end up using, some documenting needs to be done in order to understand all the functionality of the service, before jumping to implementation.
  • Even though, I assume that the most important part of this task is just to allow FWD to run in cluster mode, we would still need a minimal configuration for testing and presenting it. The easiest way would be to just have a few docker containers with the FWD server running, a reverse proxy(e.g. nginx) in front, in order to forward the requests to any of the servers. However, a more interesting solution would be to also have a platform for container orchestration(e.g. kubernetes) in order to automatically scale down or up the running containers based on the load.

Please correct me if I was wrong in any regard, or have some feedback on these notes!

#28 Updated by Greg Shah 10 months ago

The primary work would be on identifying the list of items that should be synchronized between the server instances and implementing this. I see that an initial list has been made in #4369-4. However, some things probably changed since then.

Correct.

Also, I think that this list can be split in two. Firstly, we should identify the data structures that must be synchronized in order to ensure complete functionality. Then, I would assume there will be instances where synchronization is optional, like FF cache(?).

Yes, call these lists "required" and "optional". For now our focus should be on the required list. My sense is that we should minimize the synchronization for now. FFC and other performance caches are complicated enough without having to constantly invalidate records because of remote processing. Let's reduce the complexity as much as we can.

Also regarding the synchronization between server instances, the current access to the data structures should still be preserved. We don't want to force anyone to use a 3rd party app for synchronization if they still have a single FWD server instance. This means that the access to the structures should be behind an interface in order to easily switch between single server mode and multi server mode.

Yes. By default, everything is single server and works as it does today without any extra configuration.

Regarding the 3rd party service used for sync. As I can see, Redis has been the last one mentioned here.

We have not decided yet. I am open to doing some small prototype and deciding on the merits.

My intuition suggests that since Hazlecast was originally designed for exactly this use case, it may be a better fit. I think REDIS added this concept later and may not be as natural a fit, but someone would need to actually check it.

However, a more interesting solution would be to also have a platform for container orchestration(e.g. kubernetes) in order to automatically scale down or up the running containers based on the load.

Agreed. We should ensure this works well.

#29 Updated by Andrei Plugaru 10 months ago

Greg Shah wrote:

Regarding the 3rd party service used for sync. As I can see, Redis has been the last one mentioned here.

We have not decided yet. I am open to doing some small prototype and deciding on the merits.

My intuition suggests that since Hazlecast was originally designed for exactly this use case, it may be a better fit. I think REDIS added this concept later and may not be as natural a fit, but someone would need to actually check it.

After some research, it would seem that, indeed a service like Hazlecast, which is a In-Memory Data Grid will be a better option that just a caching solution like Redis. The advantage of Hazlecast is that it, by default, can synchronize some Java collections between different server instances. As far as I understand, the 'synchronization' should actually be apparent, meaning that the data will be fetched only when explicitly requested. I would assume that still some implicit data transfer occurs in order to ensure that the entire cluster is fault tolerant. This also means that the data transmitted over the network should be minimal. Also, the work for serialization/deserialization should be minimal.

With Redis, we could also, in the end, obtain the same functionality. However, it may seem like reinventing the wheel. For example, at each update of a collection, we will have to manually be sure to propagate the change to Redis and also invalidate some local copies that there could be on other servers. There could be other things that are implicitly done by Hazlecast, or that are easily configurable.

Next, I will be working on making a demo with Hazlecast in order to understand how exactly it works.

#30 Updated by Andrei Plugaru 10 months ago

While I was going through the documentation, I discovered this page: https://docs.hazelcast.com/hazelcast/5.5/data-structures/distributed-data-structures .
Here, the data strucutres offered by Hazlecast are discussed from the perspective of CAP (Consistency, availability and partition tolerance). Based on CAP theorem, a distributed data system can only guarantee two of three properties at any given time: Consistency (all nodes see the same data), Availability (every request receives a response), and Partition Tolerance (the system continues to function despite network failures).

It would seem that the data structures they offer in the open source version offer availability and partition tolerance. This means that in the case of some nodes being disconnected from the cluster, everything will still continue to work, however, the data will obviously not be consistent with the all the other nodes, as the 2 partitions will not be able to communicate and synchronize the state. In the context of FWD, having inconsistent data in nodes could lead to really bad bahaviour as we will store here persistence related data. This network partition should not be confused with a node failing all together, as in that case even the open source data structures offer recovery. More details regarding backups on Maps can be found here: https://docs.hazelcast.com/hazelcast/5.5/data-structures/backing-up-maps .

Hazlecast actually also offers data structures that offer Consistency and partition tolerance for Enterprise edition. They achieve this by just duplicating all the data across all the nodes. This is definitely less performant than the other data structures.

At this moment, I am not sure this is a stopper for using Hazlecast as the above analysis is just theoretical. In practice, the probability of a network partition is pretty low, especially if the nodes are inside the same local network.

#31 Updated by Greg Shah 10 months ago

Consistency and Partition Tolerance is probably what we would require. Availability is less important (if a node drops out and can't respond, then the request may not get a response and that is no different from how it would work on OE).

#32 Updated by Andrei Plugaru 10 months ago

As only their Enterprise edition actually offers the strong consistency in case of a network partition, I looked over how the open source version handles the case of a network partition. It has a so called Split-brain protection:

Split-brain protection: Split-brain protection could be used when consistency is the major concern on a network partitioning. It requires a minimum cluster size to keep a particular data structure available. When cluster size is below the defined split-brain protection size, then subsequent operations are rejected with a SplitBrainProtectionException.

It requires a minimum cluster size configuration to be set. Then, it would periodically check if a network partition has occurred using heartbeats. In that scenario, it means that our original cluster is split in two. Given that the disconnecting part has less nodes than minimum cluster size, hazelcast will reject any operation to these nodes. However, the nodes in the other part will continue to work normally. The scenario this protection solves is when the same data would be overriten in both partitions. Using Split-brain protection, access to the data is allowed only in one partition, so this is avoided. However, in order to ensure strong consistency, we will actually want to deny access to any node(from both the partitions). This may be possible as apart from setting minimum cluster size, there is another programmatically way to check if we are in a situation of network partition. Here, we would need an implemenation where we check if all the cluster members are up, if at least one is down, we don't process any request. At this moment, I am not entirely sure how this would be implemented, however, I think the idea is promising.

#33 Updated by Greg Shah 10 months ago

However, in order to ensure strong consistency, we will actually want to deny access to any node(from both the partitions).

That would mean that the entire cluster goes down when any node is unavailable. That breaks the concept of partition tolerance and of availability. We can't do that.

#34 Updated by Andrei Plugaru 10 months ago

Greg Shah wrote:

That would mean that the entire cluster goes down when any node is unavailable. That breaks the concept of partition tolerance and of availability. We can't do that.

Yes, you are right, only consistency will be preserved in that case.

#35 Updated by Greg Shah 10 months ago

I will investigate the pricing and licensing of Hazelcast EE.

#36 Updated by Andrei Plugaru 10 months ago

I just found Apache Geode, it is also a in-memory data grid. On their home page they market it as Performance is key. Consistency is a must. So, I think it is also worth looking into it. :))

#37 Updated by Greg Shah 10 months ago

Andrei Plugaru wrote:

I just found Apache Geode, it is also a in-memory data grid. On their home page they market it as Performance is key. Consistency is a must. So, I think it is also worth looking into it. :))

Yes, please do look at this. I'd prefer it over a commericial option, all other things (i.e. features and performance) being equal.

#38 Updated by Greg Shah 10 months ago

The initial feedback from Hazelcast about their EE licensing requires an annual payment per node (with a minimum of 3 modes per installation). The fee per node was non-trivial and there doesn't seem to be an unlimited use license. At a minimum, this would be a significant impact because our customers would have to pay this fee (every year).

At this time, please focus attention on Apache Geode.

#39 Updated by Andrei Plugaru 9 months ago

I have continued the documentation on Apache Geode.
A first thing to mention, that I initially misunderstood is related to the general topologies of these services. Both Apache Geode and Hazlecast can be configured in peer-to-peer mode and client/server mode.
In peer-to-peer mode, all the data is stored in the heap of the same application processes. There is a minimal need for external services, maybe only for locating the processes. On the other hand, in a client/server mode, the actual data is stored in dedicated servers. The client only reaches for the server to request the data. In our case, the client would be the FWD server. The client/server mode has the advantage that it can be scaled in a different mode than the actual FWD servers. Also, client/server mode should have better stability, as if one the FWD server crashes the data will be safe. So, I would assume the client/server mode is want we want.

Next, I will focus on how the data is distributed inside a server cluster.
There are more types, however these 2 are of the most interest:
Region Type Description Best suited for...
Partitioned System-wide setting for the data set. Data is divided into buckets across the members of the cluster. For high availability, configure redundant copies so each bucket is stored in multiple members with one member holding the primary. When a key is updating, the member that holds the primary, also holds a lock on the key while distributing the updates to the members that hold copies of the keys. * Very large data sets
* High availability
* Write performance
* Partitioned event listeners and data loaders
Replicated (distributed) Holds all data from the distributed region. The data from the distributed region is copied into each member. By default, when a key is updated, this update is propagated asynchronously to the other members. Even though, there is conflict checking in place in order to solve concurrent updates, Geode only ensures eventual consistency(that means for a short period of time, it's possible that stale data will be seen on another nodes). However, there are options for setting different levels for distribution:
* distributed-no-ack. Distribution operations return without waiting for a response from other caches.
* distributed-ack. Distribution waits for acknowledgment from other caches before continuing. However, as there is no lock on the key, a stale value can still be read from some servers.
* global. Entries and regions are automatically locked across the cluster during distribution operations. All load, create, put, invalidate, and destroy operations on the region and its entries are performed with a distributed lock. IMHO, this should ensure strong consistency, however it will also come will a performance penalty as we will wait until the data is persisted on all the servers.
 		* Read heavy, small datasets
* Asynchronous distribution
* Query performance

Partition tolerance

From the documentation:

Geode handles network outages by using a weighting system to determine whether the remaining available members have a sufficient quorum to continue as a cluster.

Individual members are each assigned a weight, and the quorum is determined by comparing the total weight of currently responsive members to the previous total weight of responsive members

Your cluster can split into separate running systems when members lose the ability to see each other. The typical cause of this problem is a failure in the network. When a partitioned system is detected, only one side of the system keeps running and the other side automatically shuts down.

By automatically shutting down the part of the system that doesn't have quorum, the consistency of the data will be maintained. However, depending on the region type, some data may be lost(for replicated, no data should be lost, for partitioned - data may be lost if both the primary and and copy were in the part of the cluster that lost quorum). The loss of data can still be critical as it may lead to illegal operations.

At this moment, I think that the best option for us would be replicated region type, with the level of distribution set to global. Basically, this means that every member of the cluster will hold a copy of all the data(so we should never loose any data as long as there is at least one server running). Apart from this, strong consistency will also be maintained by having locks on the keys when updating. Unfortunately, there should also be a performance penalty for this configuration. Next, I will want to make a demo to understand this.

#40 Updated by Greg Shah 9 months ago

In regard to peer-to-peer vs client/server, the above post doesn't tell me enough to understand which way we should go. Having a separate server process to maintain adds complexity and also a potential single point of failure. Conversely, peer networking may not scale well (I'm not sure what the cluster size is where this becomes a problem).

At this moment, I think that the best option for us would be replicated region type, with the level of distribution set to global. Basically, this means that every member of the cluster will hold a copy of all the data(so we should never loose any data as long as there is at least one server running). Apart from this, strong consistency will also be maintained by having locks on the keys when updating. Unfortunately, there should also be a performance penalty for this configuration.

Agreed.

#41 Updated by Andrei Plugaru 9 months ago

Greg Shah wrote:

In regard to peer-to-peer vs client/server, the above post doesn't tell me enough to understand which way we should go. Having a separate server process to maintain adds complexity and also a potential single point of failure. Conversely, peer networking may not scale well (I'm not sure what the cluster size is where this becomes a problem).

I agree that the client/server architecture adds some complexity, however I think the SPOF concern can be easily mitigated by having a Geode cluster with more servers.
Regarding the peer to peer solution, another disadvantage I see is that each FWD server will additionally consume memory to store all the shared data across the cluster. This will, for sure, add additional memory consumption, however at this moment I am not really sure if it will be significant.

Also, I want to mention, that when I wrote the last note, in the section about client/server vs peer-to-peer, I was having in mind the partitioned region type. So, I was having some doubts regarding data integrity, as if one FWD server crashed, the data stored in that peer may have been lost. However, in a replicated region, that concern is no longer valid. Apart from that, one big advantage I see now is that the reads should be much more performant as the data would just be taken from inside the same JVM.

Also, there is the question of what happens during a network partition for a peer to peer network. This time, a network partition of the FWD servers will implicitly mean a network partition of the Geode peers. However, the same mechanism for ensuring quorum should kick in. Operations on loosing side will not be performed anymore. So, in this case, it will behave a little differently than the case with client/server architecture. In that case we didn't care if the FWD servers were in a situation of a network partition as long as the Geode servers were not in a network partition. In that case, the requests would be just redirected to Geode servers that are still available. So, theoretically, as long as there are at least 1/2 Geode servers still running the requests will still be served. In the peer-to-peer mode, when a network partition takes place for the FWD server cluster, the requests will not be served at all for the partition that lost quorum. However, the analysis in this paragraph is mainly theoretical, in practice, I assume there could be a low chance of having a network partition for the FWD servers cluster, and not having one for the Geode cluster, and both the FWD server partitions still be able to access the Geode cluster. So, it's possible that in practice, these behaviours for client/server and peer-to-peer mode would actually be equivalent.

So, given these information, at this moment, I am inclined to say that peer to peer may actually be a better solution(mainly because of the read performance), even though there is the theoretical limitation of the network partitioning. There are however, advantages and drawbacks for both architectures, however the actual implementation should be mainly independent of this config, so we can easily switch between those if we found a certain limitation or big advantage of one or another.

#42 Updated by Andrei Plugaru 9 months ago

Lately, I have investigated some approaches Geode has for propagating changes to other nodes in the cluster. By default, when a put operation is performed, the entire value is serialized and transferred to the other nodes where it should be stored. For large objects with small changes, this is inefficient because you're repeatedly sending a lot of unchanged, redundant data. I have discovered a few solutions Geode has for this problem.

Functions. Basically, classes can be defined for different tasks, they should implement Function interface from Geode and an execute method should be defined. The logic from execute will be executed on all the servers that have the needed data. The bad part about functions is that they don't really strong fault tolerance. For example, we agreed that we will set the distribution level to global. This means that when a put operation is performed a lock is being placed for that key until all servers get the new value. However, with functions this protection will be bypassed.
Delta propagation With this technique, classes that are used as values in the shared structure should implement some methods regarding the changes applied to them. toDelta/fromDelta are the most important methods. Basically, they write(read) to a stream the data that has been changed and also some markers in order to understand what exactly has been changed. At a first glance, the implementation is kind of ugly, basically, for each field we would need need another one for marking if it has been changed or not. However, maybe, it can be improved somehow. An important note here is that delta propagation can only be applied when we have the initial value in the other servers. If the server, from whatever reason, doesn't have the initial value, the whole object will be sent. The good part is that all the operations are done still through the put API so there shouldn't be any problems about consistency.

#43 Updated by Greg Shah 9 months ago

OK, it seems like we need delta propagation.

#44 Updated by Andrei Plugaru 9 months ago

I started going over the things that need synchronization based on the list from #4369-4 and came up with solutions using features from Geode:
  1. Tracking the unique indices in uncommitted transactions. The main class there is UniqueTracker, there we have this map: Map<Class< ? extends BaseRecord>, Map<String, UniqueTracker>> . However, we don't need to sync the map per-se. What we would need to sync is the instances of UniqueIndex as we have an array of them in UniqueTracker. More exactly, Map<Key, Long> records and Map<Long, Key> entries will need to be synced. Apart from these, in UniqueIndex, there is also ReentrantLock lock. Out of the box, Geode doesn't have support for this kind of locks. However, it should be able to simulate it using a map and Geode's distributed lock service Actually, I was wrong, by default locks offered by Geode are reentrant. However, now, we will need to implement by ourself locks that are non-reentrant, if we ever need those.
  2. persistence global event queue. The main class here is GlobalEventManager. The trickiest structure is SortedMap<Long, Registrant> interested. Geode doesn't have a data structure which can hold the entries sorted. However, there exists the possibility to add an index to the key of the map and execute some SQL-like queries which can mimic the behaviour of the sorted map. There is also the possibility to have a SortedMap as an entry in a Geode map. We can also have a decorator over the original SortedMap for providing the delta updates. However, the second option implies that getting a single entry will also mean always retrieving the entire sorted map.
  3. IdentityPool This class holds multiple SortedSet objects. Geode, again, doesn't provide a solution for this structure. However, the same workarounds as before can be applied.
  4. record locking The actual in memory locking mechanism is mainly based on the maps from InMemoryLockManager. They can be pretty easily synced using Geode. However, the problematic area is actually the meta_lock table. The meta table issue is discussed more in the next point.
  5. The H2 metadata tables. First, from the initial note, I would understand there are tables which should be shared between the FWD servers and ones that shouldn't. For the ones that should share the data, there could be 2 approaches:
    1. Leave the H2 in memory for each server and when a change is made, propagate it to the other servers. The communication between H2 and FWD will be still almost instantaneous, however propagating the change to the other FWD servers will for sure also impact performance.
    2. Move the H2 to server mode. This way any changes from one FWD server, will be, implicitly visible everywhere. The overall performance of the communication between FWD and H2 should be slightly worse, however there shouldn't be any other performance penalty for propagating the change to the other server instances. Overall, I think that both performance wise and from the point of view of implementation, this solution is better.
      However, if I understand correctly and there are H2 tables which shouldn't be shared between FWD servers, we should have a mechanism for those. An easy approach that I thought now is to also store a discriminator column in the non-sharable tables. However, a drawback for the server mode is that we will have single point of failure. But maybe there replication services in order to alleviate this issue.
  6. While writing the last point, I realised that, not only the metadata H2 tables need to be synced, but also 4GL temp tables that are stored in H2. However, the solutions presented in the last note should also cover this scenario.
  7. Caches. At this moment, I didn't put a lot of thought into those as FWD caches are mainly there for performance reasons, so they shouldn't be critical, at least for an initial pass.

For now, I just limited myself to the items first described in #4369-4. Eric/Greg, can you please review the points and pinpoint me to other areas that would need attention.

LE: Corrected some things in the 1st point about locks.

#45 Updated by Greg Shah 9 months ago

While writing the last point, I realised that, not only the metadata H2 tables need to be synced, but also 4GL temp tables that are stored in H2. However, the solutions presented in the last note should also cover this scenario.

I don't think this is correct. Temp-tables are session-specific data. I.e. if a session has a temp-table, that is private to the session. The design of this clustering is only to share global/cross-session state. We are not going to share session-level state.

#46 Updated by Greg Shah 9 months ago

I will also re-iterate the advice I've previously given about how we support metadata. We should NOT be maintaining the metadata in tables that can be read but instead we should build the metadata tables dynamically when accessed. We would build the metadata on the fly from whatever the in-memory state happens to be. So: for the meta_lock table, we would sync the LockManager data and then locally build the table only when it is accessed in the local server. That means we would not need to sync the state of these tables since they would be transient.

Why have I suggested this (for decades)? Because I consider the complexity of maintaining these metadata tables to be unnecessary and too much of a burden compared with the benefits. Since these tables are not used very often (most 4GL code doesn't use them), I consider the simplification and reduction in constant editing of the tables to be far better than being able to respond more quickly to the relatively rare need to access these tables.

Why take the constant performance hit of maintaining these tables? Why make our code more complicated and thus error prone/buggy? Now, to maintain this constant waste of CPU cycles, we will add to the burden and have to sync these tables across the cluster. Nasty.

#47 Updated by Eric Faulhaber 9 months ago

From #4369-4:

  • dirty share database
    • This is a quirk of the 4GL implementation where uncommitted index updates in one session are visible to other sessions. It has multiple implications that can be seen from 4GL code.
    • queries in one session can traverse uncommitted records from another session
      • Our idea at this time is that we WILL NOT support this in stateless clustering mode.
      • This is part of the changes that would be needed to make an application stateless.
    • unique constraint violation that is detected early in the 4GL because the index changes are visible across sessions
      • This part needs support.
      • For performance reasons, this is currently being rewritten to use maps.

Since this was first written, we have decided not to support this cross-session quirk/leak across sessions, instead working with customers to rewrite areas of their applications which (often unintentionally) rely on this transaction isolation leak. The implementation is flawed and cannot support all cases.

The current implementation of #8388, which is designed to replace the dirty share implementation altogether, still supports the cross-session dirty sharing (despite its flaws) under the covers. This support is needed temporarily for one project, which relies on this feature in production. That project is not expected to have a need for clustering, at least not before the application code which relies on cross-session sharing is replaced. So, we won't worry about adding the cross-session dirty sharing to the requirements for clustering.

We still need to support intra-session early sharing, but since that is within a single session by its nature, that won't be part of the clustering support.

Unique index tracking is still needed and will need to be supported in a clustered environment.

  • persistence global event queue
    • Used to notify other sessions when there are index changes.
    • This is a different aspect to the same "uncommitted index updates" problem but it also can manifest in cases where there are committed changes.
    • It is used to shift record-oriented loops (e.g. FOR EACH) from adaptive into dynamic mode.
    • This will be needed.

I think this is still a requirement for clustering, but there have been many changes to AdaptiveQuery since I last looked at this code, so I'm not 100% sure. I need to review this code.

  • identity pool
    • We no longer use recycled primary keys but we do have a small batch of pre-fetched valid IDs which are specific to the server.
    • This was done for performance instead of querying the sequence for each ID.
    • This has to be made safe.

Although we noted this as an initial requirement, I don't know if clustering is the best answer for this one. The ID-generating sequence already gives us atomicity at the database.

From FWD's point of view, I don't think it is important that different servers work with the same "shared" batches of cached primary key IDs. If each server's identity pool gets its own batches of keys from a request to the database, that should be ok; they will all be unique with respect to each other. There may be applications which rely on some quirk of how Progress allocates, recycles, or orders primary keys. However, we already don't guarantee the same internal behavior, so I think we're ok here without clustering the identity pool.

  • locking
    • The current locking approach is all in-memory.
    • We must rework lock manager, _lock metadata and the lock built-in functions.
    • In #1879 we have a potential path to moving this back into the database, which would naturally resolve this issue.
    • Another approach would be to implement a clustered version of the lock manager.

Still needed. #1879 has not been a priority and the little bit of research that had been done was only relevant to PostgreSQL. We don't yet have a clear vision of how this would work in a database-neutral way, and I don't want to hold up the clustering work to go on that detour.

  • metadata usage that is done in-memory using H2
    • _connect
    • edits to _file, _field (reads would naturally be server independent but updates must be shared)
    • check each metadata table to see which ones have editable state which must be shared

Still needed, especially if we were to re-implement metadata support per #4369-46.

  • caches
    • Several persistence data structures/classes/instances are cached for performance or memory reasons today.
    • Some of these might be safe for leaving as a per-server cache with each server having a separate (unsync'd) cache.
    • Each of the caches (dynamic temp-table definitions, queries...) will need to be inspected to determine the right approach.

As noted in the last sub-bullet, we need to take a closer look at the various cache implementations to ensure that they are only used for performance reasons, and do not have functional or other side effects.

If they are only about performance, it seems clustering them may compromise their value (this is a speculative statement; I don't know for sure). So, as previously suggested, a first pass clustering implementation can probably exclude these. If the performance benefit from caching significantly outweighs the performance penalty of clustering, we may want to support these caches in a follow-up clustering implementation.

If there are functional side effects of a cache, we'll have to consider whether clustering is needed.

I think now is the time to list each cache in the project and rule them in or out.

#48 Updated by Andrei Plugaru 9 months ago

First of all, thank you Greg and Eric for your input!

Regarding the H2 changes mentioned in #4369-44, at this moment I don't think any of those are needed as temp table records are session specific, and the metadata tables are to be shifted to be dynamically generated.

Now, about the refactoring of the metadata support from #4369-46. Even though that would be a requirement for supporting cluster mode, I see the work needed for that rather independent from the whole effort on this task. IMHO, I would rather have that done on another task in order to keep this one more manageable.

#49 Updated by Andrei Plugaru 9 months ago

I have been going through all the caches I have found in FWD in order to understand if they need synchronization:
  1. AbstractQuery.sortCache: seems to have a purely performance impact in order to avoid reparsing of sorting phrases.
  2. DynamicQueryHelper.lvl1Cache and DynamicQueryHelper.lvl2Cache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  3. FastFindCache: I am pretty sure this one shouldn't have any functional impact; the whole purpose of this is to retrieve the record identifier needed for a query, avoiding a DB trip
  4. FQLPreprocessor.translateCache: this one also seems to have only performance implications; in the only method that uses the value from the cache, if it doesn't find the needed key, it just creates and inserts the value
  5. FQLPreprocessor.astCache: same as before
  6. FQLPreprocessor.WorkArea.cacheNoArgs and FQLPreprocessor.WorkArea.cacheWithArgs: same as before
  7. FqlToSqlConverter.fqlAstCache: same as before
  8. ReportFactory.jasperReports, ReportFactory.browseReportTemplates, ReportFactory.browseReports: here the things are a little bit tricky from the point of view of invalidation. Based on this wiki: https://proj.goldencode.com/projects/p2j/wiki/JasperReports_Integration, changes in the design files can only be picked up after the old entries are invalidated. The problematic scenario I am thinking is that the request that initially creates the report is executed on one server(server A), and the request which should invalidate executes on another one(server B). So, in this scenario, server A will still hold the data which won't allow the changes in the design files to be picked up. However, at this moment, I am inclined to say that such a scenario would mean that the original code isn't really stateless, so it would need to be refactored. However, if anyone else has another opinion, I guess we can sync these.
  9. SourceNameMapper.sourceCache: In this cache the key is the propath(the hash of the propath to be more precise) and the value is the available procedure for that proapth entry. I consider that this also doesn't need cross-server synchronization.
  10. SourceNameMapper.searchPathCache: This one seems to hold a mapping from the original propath to the new one. I consider that this also doesn't need cross-server synchronization.
  11. Util.pathCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  12. BufferManager.convertedNames: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  13. DynamicValidationHelper.cache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  14. FQLHelperCache.cache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  15. Persistence.Context.staticQueryCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  16. SortCriterion.cache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  17. TemporaryBuffer.Context.fastCopyCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  18. Persister.updateCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  19. DataSourceImpl.Context.psCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  20. Session.cache: This is a very important one as it is frequently used inside the persistence layer, however as it is specific to a concrete session and we don't need cross-session leaks, I don't think we need any synchronization.
  21. AbstractGuiDriver.textMetricsCache, AbstractGuiDriver.paragraphMetricsCache: The method that retrieves the entries, also has logic for creating in case it doesn't exist, so I consider that this also doesn't need cross-server synchronization.
  22. FontTable.sharedLegacyTextMetricsCache: For this cache getMFUEntries is called. The result is sent to the client. This method returns the most frequently used items out of cache. So, it can, theoretically, return different results on different servers. However, after consulting with people more familiar on this topic, I understood that this behaviour will not be critical as the client will just compute the value again.
  23. A few caches in GuiWebSocket: however, I don't think any of these would need synchronization as they are specific to a web socket.
  24. int64.STATIC_CACHE, int64.DYNAMIC_CACHE, integer.STATIC_CACHE, integer.DYNAMIC_CACHE: these caches are purely for performance in order to cache immutable int64/ integer values.
  25. character.CHARACTER_CONSTANTS: the same as before, just that this one is for character values.

A common reason why I would think that most of these caches don't need synchronization is that they are ExpiryCache(LFUAgingCache or LRUCache), so, the records can always be evicted if the cache is full, so I would assume there shouldn't by any functionality that relies on the fact that a record really exists in the cache. An exception to this, probably is Session.cache, as the records are never evicted if they are still in use, however this one has already been ruled out for synchronization for other reasons.

The conclusion is that from the list above I don't see any cache that would really need synchronization between servers.

#50 Updated by Greg Shah 9 months ago

The conclusion is that from the list above I don't see any cache that would really need synchronization between servers.

Good, that is the plan.

I can confirm that the JasperReports, SourceNameMapper and anything related to UI (fonts/test metrics, GUI websockets) are all things that would not be synchronized.

#51 Updated by Andrei Plugaru 9 months ago

Greg, regarding the idea to move the refactoring of the metadata support to another task(as motivated in #4369-48), do you think it is ok, or should we also do that here?

#52 Updated by Greg Shah 9 months ago

Greg, regarding the idea to move the refactoring of the metadata support to another task(as motivated in #4369-48), do you think it is ok, or should we also do that here?

This is something Eric needs to decide. First we have to decide if we rework how the metadata works. I have registered my opinion above but that doesn't mean it will happen.

#53 Updated by Eric Faulhaber 9 months ago

Greg Shah wrote:

Greg, regarding the idea to move the refactoring of the metadata support to another task(as motivated in #4369-48), do you think it is ok, or should we also do that here?

This is something Eric needs to decide. First we have to decide if we rework how the metadata works. I have registered my opinion above but that doesn't mean it will happen.

It hasn't yet ;)

Andrei, as discussed earlier today, I need to understand the scope of this work, and whether it makes sense to do: (a) at all; and (b) now.

I'm not being flip when I say "at all". I think the rework would not be trivial. What we have today is arguably functional (though not ideal, for the reasons Greg points out).

The question in my mind in the context of clustering is whether the effort to rework the existing metadata support now is going to give us a significantly better, easier, more efficient clustering result.

Truth is, I'm not convinced a purely just-in-time implementation of the metadata is the right thing here. At least, the naive implementation I'm imagining would be easiest to develop (where the in-memory state of the metadata is all stuffed into an H2 in-memory table just-in-time for every query) concerns me.

The metadata (or the state it is representing) still needs to be maintained in memory and presented within an H2, in-memory database. Maintaining it in memory, as Greg notes, should be faster than constantly updating the _meta database in H2. But when it's needed, I worry about the amount of computing work necessary to stuff all that information into H2 for just-in-time use. We have to populate an entire table from scratch for each query. That might be smallish, say for the _Connect table. For the _Lock table, however, it could be huge.

And we can't expect that a program needing this information will only need it once. The records have to be prepared for each query that hits that table. We also can't assume the programs which rely on this are all utility programs which are only run occasionally. We just don't know what dependencies application developers will have built around these tables.

So, is there a hybrid approach, where maybe we track changes in metadata state, coalescing them, and only updating the H2 table with the diffs, on demand? Maybe, but that's a more complicated design. I don't necessarily want to put in the critical path of the clustering support, unless it makes the clustering support better and/or easier to implement. IIRC, we're doing something like this with the _UserTableStat VST, so maybe there's some inspiration there.

Andrei, I need your advice in this regard. As you've been doing with the various caches, please review the metadata table implementations currently in FWD which require "live" updates. These will be represented by *TableUpdater classes in the com.goldencode.p2j.persist.meta package. You can ignore the ones that don't need to draw on data that is shared across sessions.

Review the current implementations and think about whether and how these would need to be managed for clustering. You've done some of this already. Then consider how they might be implemented in more of a just-in-time design. As I noted above, I don't think we necessarily should rework the higher volume ones into a pure just-in-time implementation. But, are there lower-volume metadata tables that can be refactored now without a ton of effort, to make the clustering result better?

#54 Updated by Greg Shah 9 months ago

Truth is, I'm not convinced a purely just-in-time implementation of the metadata is the right thing here. At least, the naive implementation I'm imagining would be easiest to develop (where the in-memory state of the metadata is all stuffed into an H2 in-memory table just-in-time for every query) concerns me.

I hear the fear but don't see any evidence that there is an actual issue.

Consider that we will have to make changes to the metadata approach for this task, regardless of the approach.

#55 Updated by Andrei Plugaru 9 months ago

I have been going through the classes that update the metadata tables. Here are my findings. Both UserTableStatUpdater and LockTableUpdater already use the so called 'hybrid' approach for updating the tables. Let me give a small overview of how it is implemented. These classes hold a databaseVersion and mapVersion(version for the in-memory structure). Also the entries stored in memory also have database and in-memory versions. So, when an update is performed, the in-memory version is incremented, while the database one stays the same. At this moment, the H2 table is not updated, so it gets to be out of sync. However, this out of sync state is only until a query on the metadata table is performed. Before the query is executed, it persists to the H2 table the modified(or new) records. So, a query on these tables will never return stale data. With such a design, in a scenario with 2 consecutive queries on a meta table, only before the 1st one is executed, some time is taken for persisting the new data, however the 2nd one will have no overhead in this regard.

A different approach is for TransactionTableUpdater. Here, there are no variables to keep track of the database or in-memory version. Instead, in the flush method, a query on the metadata table is performed, retrieving the records that are already in the H2 table. And only the new ones are inserted. However, for this metadata table I observed some possible flaws. First of all, the flush method is only called from RecordBufffer.flush, even though the javadoc for the TransactionManager.flushTransMetaData(which is the only method which calls TransactionTableUpdater.flush) states that it should be called before every access to the metadata table. At this moment, I don't understand how this is called for every access given that it is only called from RecordBufffer.flush. Another flaw in TransactionTableUpdater, is that the records in H2 are never updated. Only new records are inserted here. I am not sure whether this is a flaw from the original implementation or the update behaviour was not desired.

ConnectTableUpdater and TenantTableUpdater don't seem to use anything of the above approaches. The logic for updating/deleting/creating the records in the metadata tables is processed right when it is requested.

So, it would seem that most of the metadata updaters already have the logic for persisting only the changed(or new) records right before the tables are queried. Only for ConnectTableUpdater and TenantTableUpdater, this mechansim would need to be implemented from scratch. For these, there can also be a performance hit if there is a scenario where the are a lot of unpersisted updates in the backing data structure when a query is made. However, I don't think the tenant metadata table is often updated, but I cannot say the same for the connect table. Some testing may be needed for this.

IMHO, the versioning approach from UserTableStatUpdater and LockTableUpdater is really what we need for clustering. With that, we won't need to share the metadata tables per-se, we will only share the backing data and some versioning variables in order to understand if the local H2 tables are out of sync with the global data. And from a performance standpoint, it seems pretty efficient as for each query of a metadata table, we will only process the new/updated records.

If we are to go with this versioning mechanism, I would rather, first, make the changes to work in a standalone(non-cluster) mode, test it thoroughly, and then adapt for cluster mode. Also, as it was mentioned earlier about the difficulty of such a design, I think it should be pretty straight-forward, given we are already doing that in some places. Greg/Eric, what are your thoughts on this?

#56 Updated by Eric Faulhaber 9 months ago

  • reviewer Alexandru Lungu, Eric Faulhaber added

#57 Updated by Andrei Plugaru 9 months ago

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

Update on the current status.
Up until now, I have synchronized the tracker for records in unique indices and the in memory locking mechanism. The code is in 4369a/16228. Initially, I have also started working on the persistence global event queue. The main class here is GlobalEventManager. However, at a closer inspection, it seems that it's single usage that ends up in AdaptiveQuery.execute is currently disabled because of the forceGlobalInvalidation flag that is set on false. This flag cannot be set from directory, so I don't see how the GlobalEventManager can be used unless modifying the FWD source code. Until realizing this, I have implemented some of the needed data structures, however, I have not included them in my recent commits. I have them saved on my workstation.

First, in order to enable the cluster mode, I have a added a directory config: cluster-mode under server. Then, I am checking the value of the directory config in ClusterConfig. Another config class I have created is GeodeConfig. This currently holds the address and port of the geode locator. On top of that, it also exposes some methods for creating Geode structures.

Now, about the synchronization per-se that needed to be done. For tracking the records in unique indices, I have mainly focused on synchronizing the records and entries maps and the lock from UniqueIndex.
More precisely, I have made UniqueIndex an abstract class which is implented by 2 other classes: LocalUniqueIndex and DistributedUniqueIndex. In the constructors for these classes I am instantiating the local structures(HashMap s/ ReentrantLock) or the distributed ones, and passing them to the super constructor. This way the logic in UniqueIndex remained mostly unchanged. However, some refactoring was needed to be able to have UniqueIndex.Key as the key in the synchronized map. The problem was that, up until now it was a non-static inner class. That means it was holding an implicit reference to the outer class - UniqueTracker. This implied that serializing UniqueIndex.Key would also serialize the UniqueTracker leading to some weird situations. I have solved it by making UniqueIndex.Key a static inner class. Because of that, I also had to move some methods like addToIndex and removeFromIndex in UniqueIndex.

Now, about the synchronization on the in memory lock mechanism. Mainly, I have taken the same approach as above: making InMemoryLockManager an abstract class with 2 implementations. However, here a problematic construct that is widely used is synchronized. Out of the box, it cannot work as the monitor for the objects is not shared across JVMs. I have fixed it by surrounding the critical sections with .lock()/.unlock() on distributed locks. Next, another issue was about the SessionToken which is a data member in LockStatus class. Up until now, instances of SessionToken were really unique per session. So, equality of these objects could be checked with ==. However, as instances of SessionToken are inside instances that are synchronized with Geode, updating some member in LockStatus(and re-inserting in the holder map) will make that other instances of SessionToken will be created, even if they semantically refer to the same one. So, checking the equality with == cannot work as we will get false negatives. For now, I have implemented equals/hashCode in this class and used these instead of the ==. However, if there is some invariant in FWD that we cannot have multiple instances of SessionToken per same session, let me know and I will try to rethink this part. Next, a problem I still haven't solved in this area are the constructs .wait()/.notify(). Out of the box, the notify signal won't be sent across the JVMs, so a wait call will stuck. The good part is wait is called with a timeout parameter, so it will still return after the time elapses. I will try to emulate these constructs with Geode's structures.

For now, I would want to get an initial review/feedback on the current implementation, especially on the architecture and how and what I synchronized in UniqueIndex/InMemoryLockManager. Please keep in mind there could be still some code styling issues/ missing javadocs.
I will also come soon with a post describing the docker setup I made for testing the cluster mode.

#58 Updated by Greg Shah 8 months ago

  • Status changed from WIP to Review
  • reviewer Greg Shah added

Eric/Alex: Please do this first review.

#59 Updated by Alexandru Lungu 8 months ago

Queuing it up!

#60 Updated by Alexandru Lungu 8 months ago

Review of 4369a:

Model

  • I spot some changes in the synchronization in general and I want to better understand the synchronization model to provide consistency. I could not spot this in the discussion in #4369 properly:

Within single-server mode and cluster mode, the server should still handle multiple requests in parallel? I mean, if there are 1k requests at a time and 10 servers, it is completely valid to balance 100 across these 10 servers? Or does it mean that servers become single-session and a reverse proxy will handle the throttling of requests?

My worry is that:

  • GeodeSequenceGenerator or/and DistributedSequence are not synchronized. If my understanding of clustering is that multiple sessions can still access the same server, then the code should still be synchronized for these cases. For instance:
    • counterRegion.get(sequenceName) can provide the same value to all requested (from within a server)
    • counterRegion.replace(sequenceName, currentValue, newValue) may not be therad-safe. One single server can have 2 sessions doing the replace at the same time (not two distinct servers).
  • DefaultDirtyShareManager should not have the possibility to use a distributed lock manager. Keep it InMemoryLockManager.

Good stuff

  • Most of the synchronized blocks are rewritten to ....lock(); and ....unlock();. I think this is fine considering that the critical blocks should be synchronized across all servers. This is nice.
  • Also, the interface / abstract class model is really nice.

Finally, I am a bit concerned of the mash-up between single local resource vs clustered resource vs single remote resource (see RemoteLockManager, RemoteMultiplexer). Greg/Eric is the "remote" model still in play for this clustered mode. Should clustering should also consider the possibility to have the lock manager remote? Or should it replace it? I expect RemoteLockManager to delegate the lock managing to another FWD server (master), but what does this means in clustered mode? Isn't Geode replacing this entirely? My point is that maybe we should purge the remote stuff together with this effort of #4369.

Small notice: FastFindCache has a functional impact as it "leaks" updated records. If one session updated a record and then FINDS it, then the cache will be updated and the record can be found by other sessions. For instance, if you run FIND FIRST in session 1 and find an updated record, running FIND FIRST in another session will also find the updated record., although in the second session that record may not have been FIRST. The data per se is not leaked, but its position (FIRST/LAST) is leaked. AFAIK, dirtyCopy records (that are created/updated and not flushed) are not saved in FFC.

#61 Updated by Andrei Plugaru 8 months ago

Within single-server mode and cluster mode, the server should still handle multiple requests in parallel? I mean, if there are 1k requests at a time and 10 servers, it is completely valid to balance 100 across these 10 servers? Or does it mean that servers become single-session and a reverse proxy will handle the throttling of requests?

A single server should still be able to process multiple requests from different sessions. The reverse proxy is also in plan in order to forward the requests to different servers(probably based on the load). However, we will need to take care that requests from the same session always land on the same server.

My worry is that:

  • GeodeSequenceGenerator or/and DistributedSequence are not synchronized. If my understanding of clustering is that multiple sessions can still access the same server, then the code should still be synchronized for these cases. For instance:
    • counterRegion.get(sequenceName) can provide the same value to all requested (from within a server)
    • counterRegion.replace(sequenceName, currentValue, newValue) may not be therad-safe. One single server can have 2 sessions doing the replace at the same time (not two distinct servers).

In GeodeSequenceGenerator.getNext, I am using the compare and swap technique. Basically, I am first, retrieving the entry, then calling counterRegion.replace(sequenceName, currentValue, newValue). The replace method behaves the same as the one from ConcurrentHashMap. It first, checks that the current value in the map equals to currentValue. If the condition holds, updates it with newValue. If it doesn't, replace method returns false. According to the documentation, the operation is performed atomically. By atomically, I would assume it means also inside the same server, not only cross server. Getting back to GeodeSequenceGenerator.getNext, if the call to replace returns false(i.e. another session either from the same server or from another server modified the value in the mean time), just redo the logic, until we are able to change the value. Using this technique, there is no need to use an explicit lock, even though they probably still use some kind of locking mechanism inside, but probably they are doing that more efficient.

DefaultDirtyShareManager should not have the possibility to use a distributed lock manager. Keep it InMemoryLockManager.

Got it.

#62 Updated by Greg Shah 8 months ago

I don't think we need the RemoteLockManager if we have proper clustering.

#63 Updated by Greg Shah 8 months ago

if there are 1k requests at a time and 10 servers, it is completely valid to balance 100 across these 10 servers?

If the requests are stateless, then they should be able to be distributed across the clustered servers without any constraint. But they requests must truly be stateless.

Or does it mean that servers become single-session and a reverse proxy will handle the throttling of requests?

Not this.

#64 Updated by Alexandru Lungu 8 months ago

If the requests are stateless, then they should be able to be distributed across the clustered servers without any constraint. But they requests must truly be stateless.

So a single server is still handling multiple requests at once (on different threads). I was inclined to say so; in this case all distributed access should be synchronized within one server (unless Geode allows thread-safe classes itself).

#65 Updated by Andrei Plugaru 8 months ago

I have mainly finished all the changes necessary in InMemoryLockManager regarding the .wait()/.notify() constructs. In the end, I replicated them using a listener on the distributed structure. The listener has an object for each key we are interested in, which acts as a monitor. However, after I have implemented that, during testing, I was encountering random hangs of the server. In the end, it seems the root cause is that the threads were previously interrupted(probably during the current logic in FWD) and it was entering some loops inside Geode's code which it was exiting after a long period of time. I have managed to found some workarounds that seem to work for most of the cases, however I don't really think this should be the intended behaviour. I have managed to also replicate this in a standalone application, so I intend to open a ticket on their platform for tracking issues.

The current revision of 4369a is 16231. Apart from the improvements mentioned above, it also contains some refactoring and added documentation.

About the concern mentioned in the last note:

I was inclined to say so; in this case all distributed access should be synchronized within one server (unless Geode allows thread-safe classes itself).

This is from Geode documentation(https://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/package-summary.html):

"Global" scope provides the highest level of data consistency by obtaining a distributed lock on a region entry before propagating a change to other members of the distributed system. With globally-scoped regions, only one thread in the entire distributed system may modify the region entry at a time.

As we are always using the global scope on the regions, I don't think we need any other synchronization besides the implicit one from Geode.

#66 Updated by Andrei Plugaru 8 months ago

  • % Done changed from 20 to 30

I have committed 4369a/16232. I have completed all the java docs, history entries all the file headers on the new files.

Currently, out of the list of things that need to be synchronized across the servers the single one remaining is the metadata tables. I have done some analysis on how, currently, the metadata tables are updated, back in #4369-55. My conclusion back then was that the 'hybrid' approach to update the tables would fit the cluster work. Basically, before each query(to a metadata table), we would populate the H2 with the data that was added after the last time it was populated. This approach is already in some tables, the only ones that would need to be implemented from scratch are ConnectTableUpdater and TenantTableUpdater. Right now, it came into my mind an ever more 'hybrid' approach: to also make updates to the H2 tables when we add data in the FWD backing structure when we have more than X elements stored there. This way we will avoid scenarios where huge amount of records would need to be inserted into H2 by keeping at most X elements.

As I also mentioned during the yesterday meeting, Greg/Eric, I would need your input on this: is this 'hybrid' approach good enough or should I think of another solution for this metadata tables problem?

#67 Updated by Andrei Plugaru 8 months ago

In the last note, I forgot to mentioned the new directory configurations introduced in my latest commits. Right under server node, I added cluster-mode. This should specify if the server should run in cluster mode or not. If this configuration is not found, the default value is, obviously, false. Apart from this, there is geode/locatorHost and geode/locatorPort. Just to be noted, the locator role in a peer to peer configuration is to act as a discovery mechanism.

However, while I was writing this note, I thought about other configuration that will be needed. Up until now, I was running the servers on my local machine, so the servers could easily communicate using the default ephemeral ports(without having problems with firewall, for example). However, in a deployed situation, we would probably need to specify a specific port(or range of ports) on which Geode will communicate in order to have only that exposed.

#68 Updated by Eric Faulhaber 8 months ago

Andrei Plugaru wrote:

[...]
IMHO, the versioning approach from UserTableStatUpdater and LockTableUpdater is really what we need for clustering. With that, we won't need to share the metadata tables per-se, we will only share the backing data and some versioning variables in order to understand if the local H2 tables are out of sync with the global data. And from a performance standpoint, it seems pretty efficient as for each query of a metadata table, we will only process the new/updated records.

If we are to go with this versioning mechanism, I would rather, first, make the changes to work in a standalone(non-cluster) mode, test it thoroughly, and then adapt for cluster mode. Also, as it was mentioned earlier about the difficulty of such a design, I think it should be pretty straight-forward, given we are already doing that in some places. Greg/Eric, what are your thoughts on this?

I agree with the general decision to model this in principle after the UserTableStatUpdater and LockTableUpdater designs. I've only looked into the LockTableUpdater code recently, but I take your analysis of UserTableStatUpdater at face value.

However, the current logic with this implementation assumes that, at any given time, there is one database representation of the state and one in-memory representation of the state, and the relative level of versions between these determines what the sync process must do.

We have to make sure that there are no side effects in the current implementation which will break things when that assumption no longer holds. I don't think this is the case right now. The sync process appears to be lossy, as it relates to the in-memory state.

What I am referring to is some code which removes items from the in-memory state during the database sync operation. This is by design, AFAICT, to keep the in-memory state from leaking memory over time. If there are multiple nodes in the cluster, each with independent database state, this seems like it will corrupt the clustered, in-memory representation of state. Consequently, this will corrupt every node's database state over time.

Thus, sharing the in-memory state based on the current LockTableUpdater implementation will not be enough, without further changes to account for the design change from one-to-one to one-to-many, w.r.t. in-memory state <-> database state. So, I think making the implementation work in a standalone (non-cluster) mode is not enough as the basis for a clustered implementation. Clustering requires that differences in the model need to be considered from the outset. The sync process of the in-memory state to any one database node cannot damage the clustered in-memory state.

That complicates things a bit, because we cannot just let the in-memory state grow forever. The original implementation of LockTableUpdater relies on the one-to-one nature of the model to assume that cleanup of the in-memory state is safe during sync. It is safe, when there's only one database to sync, as in the current implementation. It gets more complicated when there are multiple database nodes, each at different versions. In addition, the most common, non-clustered mode must remain as efficient as possible.

#69 Updated by Greg Shah 8 months ago

I repeat my points of #4369-46 and #4369-54. The existing approach to metadata is overcomplicated and causes unnecessary CPU usage maintaining internal database entries that may never be read or accessed. Now, we have to make it even more complicated. It is a terrible idea. There was no evidence that we needed to over-engineer and over-complicate this in the first place. Now we are going to make it worse, because we pre-optimized due to "vibes".

My approach would only sync the backing data. For example, we have to sync the lock data anyway. We already have all the data we need to create the correct metadata entries on any node. And this is only needed in the very rare case that some 4GL code actually needs to read the _lock table.

#70 Updated by Andrei Plugaru 8 months ago

Greg Shah wrote:

I repeat my points of #4369-46 and #4369-54. The existing approach to metadata is overcomplicated and causes unnecessary CPU usage maintaining internal database entries that may never be read or accessed. Now, we have to make it even more complicated. It is a terrible idea. There was no evidence that we needed to over-engineer and over-complicate this in the first place. Now we are going to make it worse, because we pre-optimized due to "vibes".

Greg, just to be clear, do you suggest to give up on the H2 metadata tables all together?
This would mean to create the DMOs based from the in memory data. However, the tricky part would be to handle the WHERE clause as we would need to do the filtering based on the in memory data.
However, if we can solve that, we would have the advantage to not duplicate the data both in H2 and in the FWD structures.

#71 Updated by Greg Shah 8 months ago

Greg, just to be clear, do you suggest to give up on the H2 metadata tables all together?

No. There is value in allowing the converted 4GL code just work naturally.

Some of these tables are static. For example, _file or _field have unchanging contents that are fixed at the moment the server starts. These tables never need edits and don't need to be synchronized. We should create them once and leave them alone.

For the tables that need to represent state that can change at runtime, I am suggesting that the current state in the H2 metadata tables should be emptied and recreated at the moment that a new query of the associated table occurs in converted 4GL code. If the converted code ever queries the _lock table (or whatever), we wouldpause the query before it accesses the table, fill the _lock table from the current state of the lock manager and then let the query use that table. It would be up to date at that moment and we would not spend any time maintaining every lock state change as an update to the _lock table.

My point here is that most (if not all) of the metadata usage is rare in actual production code. The _lock table is a classic one. Many applications will have some 4GL code that does reference that table, but that code is for their technical support group and is not used by normal users. If we are lazy and only build the dynamic tables when we know the query needs the updated state, then we have a potential improvement in performance by not doing all the extra maintenance. And as bonus: our code is much simpler and less fragile.

#72 Updated by Andrei Plugaru 8 months ago

Greg Shah wrote:

For the tables that need to represent state that can change at runtime, I am suggesting that the current state in the H2 metadata tables should be emptied and recreated at the moment that a new query of the associated table occurs in converted 4GL code. If the converted code ever queries the _lock table (or whatever), we wouldpause the query before it accesses the table, fill the _lock table from the current state of the lock manager and then let the query use that table. It would be up to date at that moment and we would not spend any time maintaining every lock state change as an update to the _lock table.

My point here is that most (if not all) of the metadata usage is rare in actual production code. The _lock table is a classic one. Many applications will have some 4GL code that does reference that table, but that code is for their technical support group and is not used by normal users. If we are lazy and only build the dynamic tables when we know the query needs the updated state, then we have a potential improvement in performance by not doing all the extra maintenance. And as bonus: our code is much simpler and less fragile.

I understand your idea. However, for the _lock and _UserTableStat tables, we already have a mechanism for updating the H2 tables only before a query of the associated table occurs in converted 4GL code. In this context, the update means inserting the records that were added after the previous update. This mechanism needs a version for the in-memory data and the database one. This is the so called 'hybrid' approach, I was mentioning in the previous notes. Over the solution presented by you, this has the advantage that the tables don't need to be recreated before each query. For 2 consecutive queries over a metadata table(assuming no changes to the metadata have been done in between the queries), we will need to insert data into H2 only before the 1st query. However, this mechanism has the disadvantages of using memory for the H2 records(if I understood correctly, in your approach, we would empty the metadata table after the query) and a more complicated implementation.

Now, I would put the following question: are the queries to the metadata tables really so rare that it's more efficient to just insert ALL the records before a query(and remove them afterwards) than using more memory for the H2 records in order to keep the records (that were inserted before the last query) persisted? Also, maybe it is worth doing some experiments in order to understand really how much time does it take to insert a large number of records in a H2 table. This may help us get to a conclusion.

Also, about concern exposed in #4369-18, about UserTableStatUpdater and LockTableUpdater. I have looked in both classes and couldn't really find any logic for removing the records from the in-memory structures after they are persisted to H2. The only logic for deleting the records from the in-memory structures is triggered by actual removal based on OE logic(e.g. the release of a lock on a record). A reason why the removal from the in memory structures is not performed could be the updates to the record. Both UserTableStatUpdater and LockTableUpdater support updates to the in-memory records. If the records would have been deleted, we would need to retrieve them from H2 before performing the update. However, there could still be a problem in a clustered environment because of the removal of the in memory record after an OE event as the removal would need to be propagated to the other H2 databases.

#73 Updated by Greg Shah 8 months ago

Over the solution presented by you, this has the advantage that the tables don't need to be recreated before each query.

I don't see why the tables themselves need to be deleted. We would just clear the contents.

The only advantage of this hybrid approach is that it might have less work to do to update the table at the time of the query. But it comes with a cost of burning CPU cycles on the constant maintenance of those diffs. I don't see how the metadata is used enough to justify the extra cost of that CPU usage.

are the queries to the metadata tables really so rare that it's more efficient to just insert ALL the records before a query(and remove them afterwards) than using more memory for the H2 records in order to keep the records (that were inserted before the last query) persisted?

It is worth considering. I think some of these tables are never accessed at all over the average lifetime of the server. The _lock table would certainly be one of those. Consider this: none of these tables is going to hold millions of records, There are only so many locks or users or connections that are active in a system at one time. Why take a constant hit on every edit? Inserting a few tens or hundreds of records seems to be a small price to pay to avoid all the constant maintenance.

#74 Updated by Andrei Plugaru 8 months ago

Today I have made some experiments for understanding the time taken for inserting a large number of records in a metadata table. I have created 5000 records in _lock table, and the time taken for inserting all of them was 60-100ms. An interesting observation while I was running the query on the _lock table with 5000 records, is that it was calling the method to persist the records to H2 from the in-memory structure multiple times. That's because the method to persist the records is called before each SQL query. In FWD, the AdaptiveQuery turned to use ProgressiveResults, so it was actually performing more SQL queries - for each bracket. However, as currently the _lock table uses the 'hybrid' approach, the method to persist records short circuited based on the versions. So, this is another thing to keep in mind - a single OE query could lead to multiple persist method calls. This may be fixable by moving the call to the persist method earlier in the call stack, however this needs more investigation. Another approach will be to keep some versions for the in-memory structure and H2 and short circuit the execution just as currently it is done. However, this will already be getting close to the hybrid approach :)).

In the end, it all comes down to the number of queries executed for the metadata tables for the current and future customers. Let's say there are 10 queries on a metadata table, with 5000 records. This means 600-1000ms, in a scenario where we populate the table before each OE query. However, as I stated in the previous paragraph, that is not the case out of the box. So, there could be a 4x-5x time increase - 2400-4000ms.

In the hybrid approach, this big hit will be taken only for the 1st query. However, we will constantly need to increment and distribute some counters that will serve as the versions. However, if implemented efficiently I don't really think this can cause a big performance hit.

However, I just thought of something else: creating some new directory configs for disabling the usage of each metadata table in the case they are not needed at all. If that config would be enabled, we will obviously not execute any logic in regard to store or updates of the entries. Together with the 'hybrid' approach, maybe this can provide the best middle ground.

#75 Updated by Greg Shah 8 months ago

In the end, it all comes down to the number of queries executed for the metadata tables for the current and future customers. Let's say there are 10 queries on a metadata table, with 5000 records. This means 600-1000ms, in a scenario where we populate the table before each OE query. However, as I stated in the previous paragraph, that is not the case out of the box. So, there could be a 4x-5x time increase - 2400-4000ms.

Why would we do this? We do not need to refresh the table between every call to the database. We would populate it once when the FWD query instance is opened and that is it. Multiple related queries would just access the same unchanged table. In other words, one query from the 4GL perspective should just see one static data set. The fact that FWD translates this into multiple queries is not a reason to refresh in between each one.

Also: 5000 locks seems pretty big. The vast majority of the installations will not have anything like that. Even with 5000 locks, 60-100ms is no problem for population of a table that is almost never used. Metadata is not something we should be worried about optimizing.

Hybrid just doesn't make sense. Why are we wasting time on trying to make a bad idea live on?

#76 Updated by Alexandru Lungu 8 months ago

I am just writing here a discussion that happened in the daily yesterday.

Back when we tested performance for a large customer POC, we used 7156b that did not hit a procedure that did work with _File and _Db (named GetDatabaseMetadata.p). In short, that procedure was meant to called at POC testing (once per run) and gathered all _file data per _db into a temporary table. I can't tell why, but maybe it was just caching stuff. Later, when we tested 7156c (and ultimately 7156d and 7156e) that enabled the execution of GetDatabaseMetadata, the time decreased with ~22%.

  • with GetDatabaseMetadata: 7156b (Java 8) it was working in 8.740s and 7156e (Java 17) in 9.657s.
  • without GetDatabaseMetadata: 7156b (Java 8) was working in 8.664s and 7156e (Java 17) in 7.466s.
  • 7156b was mostly the same, but 7156e's performance improvements were not only nullified, but also slowed by the metadata work.

So, GetDatabaseMetadata was running in 2.2s and all it did was to gather the _File data in a temporary table. The hotspot was an actual FOR EACH that iterated _File. In the end, the customer dropped the "optimization" completely in the later code drops. Maybe it was faster to work with _meta directly instead of caching it in _temp and fact a constant 2.2s lag. I suspect that OE was working way better with that.

My point: whatever the decision regarding _meta will be, please consider the performance implications. Apparently, there are valid customer use-cases where _meta is interrogated and whole seconds are wasted. Some tables are smaller and queries seldom, but a real customer scenario (even a POC) shown a use-case where a large table of _meta (_File) was iterated from first to last - FWD was extremely slow already.

#77 Updated by Greg Shah 8 months ago

The tables we could expect to be used frequently are not the same tables that are editable. _file, _field, _index... are all static tables. They are not subject to the overcomplicated "hybrid" mode. So I don't think this is a reason to decide things differently. That POC would have been unaffected by my proposals.

Kill. Hybrid. Mode. Now.

#78 Updated by Eric Faulhaber 8 months ago

Greg Shah wrote:

The tables we could expect to be used frequently are not the same tables that are editable. _file, _field, _index... are all static tables. They are not subject to the overcomplicated "hybrid" mode. So I don't think this is a reason to decide things differently. That POC would have been unaffected by my proposals.

Kill. Hybrid. Mode. Now.

Andrei, please work on another aspect of clustering. Greg and I need to discuss this offline. I do not agree with this assessment, and this is my area of the code.

#79 Updated by Andrei Plugaru 8 months ago

I have been lately focused on implementing refactoring the LockStatus/TraceLockStatus classes in order to be able to take advantage of Geode's delta propagation. Basically, with delta propagation Geode avoids sending the full objects other to the other members. Instead, it sends only the updates made. Of course, if the other members don't have the object at that moment, it will send it completely.

The classes that want to use delta propagation need to implement Geode's Delta interface. The most important methods are fromDelta/toDelta. They will handle the serialization and de-serialization of the updated fields.

In our case, the fields that were modified in LockStatus are LockType, singleLocker/lockers map and lockAdminTime/lockWarnTime. In TraceLockStatus, traces map is modified. But where exactly this new logic for tracking and propagating the changes should reside? I started with an inheritance strategy but got blocked because the distributed version of TraceLockStatus needed to inherit behavior from both the standard TraceLockStatus and the distributed version of LockStatus. Since Java does not support multiple inheritance, this was not possible. I then moved to the decorator pattern, creating distributed versions of TraceLockStatus and LockStatus. The constructors for these new classes receive instances of the non-distributed versions to delegate most of the logic, allowing me to simply add tracking logic to the methods that modify state. Although this required a new interface to expose common methods and added a bit of boilerplate, it is the cleanest approach I could come up with.

I have committed all these changes in 4369a/16235.

#80 Updated by Greg Shah 8 months ago

I'll have to think about the pattern a bit more before I discuss that.

I do want us to avoid putting the same Geode-specific logic in lots of classes. I'd prefer if we implement the core "types" that we need to synchronize as a set of classes in a clustering-specific package. For example, if we have HashMap instances to synchronize, we would implement a HashMap compatible class that hides the Geode implementation inside of it. At runtime we would instantiate the clustering or the normal variant in some factory that knows about the directory configuration.

This way, adding clustering to a given class could be done as simply as changing how we instantiate the core data structures. Everything else would be hidden in the clustering package (and any setup code for the server itself).

#81 Updated by Andrei Plugaru 8 months ago

The core data structures we need are mainly already implemented by Geode. They have Region, which is basically a distributed map. It implements the Map interface, so it can be plugged in easily. There are also distributed locks which can be used. However, there are other Java functionalities used in FWD that cannot be integrated in the data structures classes. For example, a standard Java pattern that I saw in InMemoryLockManager was to retrieve a value from a map, pass it to some methods that would do some update operations on that. That obviously works in Java, as we are working on the same object that is stored in the map. However, that cannot work with Geode's Region. Any update to an object retrieved from it won't be propagated to the other members without explicitly calling put. On top of that, synchronized blocks had also to be refactored. As they use the object's monitor which cannot be distributed by Geode, I had to wrap the code inside the synchronized blocks with .lock()/.unlock().

Furthermore, we cannot take advantage of the delta updates, without adding logic(to track updates) in the class acts as the value in the distributed map. We just cannot obtain it only with changes to the type agnostic collection classes.

I have implemented all of the above currently in 4369a. I have tried to leave the current existing classes free of Geode logic. I have achieved this by extending the current classes or by applying decorators, however I kept the new classes in the same packages as the base ones. What I want to emphasize is that just using Geode's structures isn't enough to enable clustering as we need some small behaviour changes in the business logic.

#82 Updated by Greg Shah 8 months ago

I understand that the logic in various places will need to be changed to be compatible with a synchronization approach. That is expected.

Please still work to put as much of the Geode-specific logic into a clustering package. If possible, I would not want any references to Geode itself outside of that package.

#83 Updated by Andrei Plugaru 8 months ago

Greg Shah wrote:

Please still work to put as much of the Geode-specific logic into a clustering package. If possible, I would not want any references to Geode itself outside of that package.

Got it. Will do this.

#84 Updated by Greg Shah 8 months ago

  • Status changed from Review to WIP

#85 Updated by Andrei Plugaru 8 months ago

I have rebased the 4369a branch to trunk/16298 and made a few commits for moving the distributed specific logic to separate packages. The current revision of 4369a is 16310.
Lately, I also worked on having converted and running a sample rest API project. Then, I created a docker image based on my local project. Based on my current docker compose file, it is actually fairly easy to spin up multiple containers for the same service(FWD server in our case), using just --scale <serivce name>=<number of instances>. On top of that, I have configured an NGINX container which forwarded all the requests to one of the servers based on round robin strategy. In this architecture, the client only makes requests to the NGINX server which acts as a load balancer. However, this was only a simple POC for REST requests, for PASOE other configuration may be needed(or not?). However, I assume more work on the load balancer will be done on #5170.

However, getting back to the work that needs to be done here. I have been thinking on a safety feature for the FWD server. Even though, a requirement for the servers running in a cluster would be to use the same p2j and application jars. I would think that this condition, may accidentally be violated by some clients, which could lead to really bad behaviour, which can be really hard and difficult to investigate. I am thinking of a functionality to check, at server startup, if the same jars are being used as in the other servers and log if there are some differences. At this moment, I am not exactly sure, how to implement this, but I think it can be a nice addition to the work here. Greg/Eric, what do you think about this? Do you think is it necessary?

#86 Updated by Greg Shah 8 months ago

Comparing the FWD version number and even a checksum of the jar would be a reasonable thing at server startup in cluster mode.

#87 Updated by Andrei Plugaru 7 months ago

The functionality described below is in rev. 16311 and 16312.

I have implemented the FWD version(and a hash of the p2j jar) checking when a node is connecting to the cluster. My initial idea also consisted of checking the jar(s) with the converted application. However, at this moment I didn't find any way of determining in the runtime which jars had the conversion. I am open to ideas in this regard.

While checking the above functionality, which included stopping containers with the FWD server running, I was getting into the following behaviour. After I stopped a container with the FWD server, Geode was getting into a network partition situation as the other node wasn't responding to heartbeats. This could even get to a total cluster shutdown if the stopped node was having higher member weight. This is obviously not the desired behaviour as this wasn't a network partition, this was just a node being intentionally stopped. I partially fixed this by setting a shutdown hook, where I disconnect from the cluster. This worked when I start the server locally, however, it doesn't work in my Docker environment. Currently, the entrypoint in my container is server.sh script. As it is currently implemented, the server.sh script creates another process with the FWD server. As a consequence, when the SIGTERM signal is sent to the entry point, it won't forward the signal to the FWD server, making it zombie until Docker kills it forcefully. In this case the shutdown hooks are not called at all. I think the easiest solution for this would be to run exec inside the server.sh. This way the Java program will just replace the process running the bash script. This will allow the kill signals to be sent to the FWD server.

IMHO, clients using the clustering feature, will likely use it in a docker env, where they will stop and start containers based on the load, so I think allowing the FWD server to be gracefully stopped when the container is stopped is a necessary addition.

#88 Updated by Greg Shah 7 months ago

IMHO, clients using the clustering feature, will likely use it in a docker env, where they will stop and start containers based on the load, so I think allowing the FWD server to be gracefully stopped when the container is stopped is a necessary addition.

I agree. Moreover, graceful shutdown is a core requirement in any installation environment, not just Docker.

#89 Updated by Andrei Plugaru 7 months ago

Greg Shah wrote:

I agree. Moreover, graceful shutdown is a core requirement in any installation environment, not just Docker.

As the change I proposed(adding exec in the command from server.sh) isn't a FWD change per-se, should I document it anywhere or what should be the steps?

#90 Updated by Greg Shah 7 months ago

As the change I proposed(adding exec in the command from server.sh) isn't a FWD change per-se, should I document it anywhere or what should be the steps?

We have a standardized version of server.sh which is the core script across all customer projects. We need to test the exec change thoroughly enough to be confident it is safe for all applications. Then the change can be pushed independently of the clustering branch.

#91 Updated by Greg Shah 7 months ago

Please work with Roger to evalute and organize testing the exec change.

#92 Updated by Roger Borrello 7 months ago

Greg Shah wrote:

Please work with Roger to evaluate and organize testing the exec change.

I must admit I read way more of this task than was necessary for my input.

The server.sh invokes java and still has some work to do before exiting because it can be invoked in order to just display the command that will be executed ($test -eq 1), determine if the FWD server is currently running ($statcode = "y"), or really starting the server:

# Ensure UTF-8 encoding
original_lang="${LANG}" 
ensure_utf8_lang

# run the FWD server
if [ $test -eq 1 ] ; then
   echo $prog $dump $perf $hprof $maxheap $srvr $jmx $dtxt $agent $spi $open_api $cpath $cdsparams $entrypoint $mode $batch $cfg $port $profile "$@" 
elif [ $statcode = "y" ]; then
   $prog $dump $perf $hprof $maxheap $srvr $jmx $dtxt $agent $spi $open_api $cpath $cdsparams $entrypoint $mode $batch $cfg $port $profile "$@" 2> /dev/null
else
   eval $prog $dump $perf $hprof $maxheap $srvr $jmx $dtxt $agent $spi $open_api $cpath $cdsparams $entrypoint $mode $batch $cfg $port $profile "$@" $output_redir
fi

rc=$?

# Restore the original LANG setting, if sourced and set
[ "${BASH_SOURCE[0]}" != "${0}" ] && [ ."$original_lang". != ".." ] && export LANG="$original_lang" 

if [ $statcode = "y" ] && [ $test -ne 1 ]; then
   case $rc in
      0  ) status="STATUS_RUNNING" ;;
      1  ) status="STATUS_TIMEOUT" ;;
      2  ) status="STATUS_UNKNOWN" ;;
      3  ) status="STATUS_STOPPED" ;;
   esac
   echo "Result =" $status
fi

# Check if the archive was created successfully when appcds=2
if [ $appcds -eq 2 ] && [ ! -f $cds_archive ]; then
   echo "Warning: Archive $cds_archive was not created successfully." 
fi

It may need to cleanup after "Application Class Data Sharing" as well.

That being said, to invoke java using exec is a good idea so that it becomes PID 1, but only in that specific case. Should it always be used, even in non-Docker situations? It can be protected with a check for [ -f "/.dockerenv" ] to restrict it.

The situations where docker compose are the most important ones to ensure exec is used, because the docker run typically has another method to hold the sessions open, like the coordination between hold_session.sh and start_server.sh, which is not applicable to this clustering use case.

#93 Updated by Greg Shah 7 months ago

This has nothing to do with Docker. It needs to always be done (bare metal, Docker, VM... whatever). If there is a problem with the exec, then we can try to limit it to the cluster case BUT I prefer if we always use it.

The $test scenario can be ignored since we would not use exec in that path.

#94 Updated by Andrei Plugaru 7 months ago

I found another mechansim to handle the pass of the KILL signal to the java process that should also allow execution of the remaining logic in the server.sh script.
Basically, we shall start the java process in background with something like this:

eval exec $prog [...] &
pid=$!
echo "Waiting for process: $pid" 
wait "$pid" 
echo "Java has finished. Running post-execution steps..."

Apart from starting, the process in background, it will also store the PID in pid variable and wait until it exists.
However, before starting the Java process, we should trap the signals we are interested in and define the handler:
shutdown_handler()
{
    # Send SIGTERM to the Java process
    kill -SIGTERM "$pid" 

    # Wait for the Java process to exit gracefully
    wait "$pid" 

    echo "Java process exited." 
}
trap 'shutdown_handler' SIGTERM SIGINT

This way we will send the same signal to the Java process.

In a docker container this will work out. Even though, the process for server.sh will have PID 1, it will 'redirect' the signal to the FWD server, allowing it to gracefully shut down.
NOTE: For the 1st shell snippet, I have also experimented without exec, as I also expected it to start the Java process in background. However, in reality, besides the Java process, it was starting a new process for server.sh and pid variable was having the id of that process instead of the Java one, so I wasn't getting the desired behaviour.

#95 Updated by Roger Borrello 7 months ago

I think there are some problems there. eval exec $prog [...] & is contradictory because:
  • exec replaces the current shell process
  • & forks a background job
I don't think this will behave as you desire, since the bash will:
  1. Fork a subshell for the background job
  2. In that subshell, exec replaces that subshell
  3. Your original shell continues running
    So:
    • $pid=$! → PID of the background subshell (now replaced by Java)
    • shell still exists
    • trap exists in the parent shell
    • Signals sent to the container / service may not reach Java

Have you been able to test this out? I've heard the term "signal-forwarding init shim" used for what we are trying to do (make sure our server.sh bash gets the termination signals for proper shutdown) but often there are more than one java launched that are not children of each other, like when there are app servers or web clients.

#96 Updated by Andrei Plugaru 6 months ago

I resumed the work here by first rebasing the branch :))
4369a has been rebased from trunk/16364. It is now at revision 16378.

#97 Updated by Andrei Plugaru 6 months ago

Roger, regarding the forwarding of the KILL signal to the Java process. I think I got to a cleaner approach using only eval and a background job. Last time I needed also the exec command as only this command eval $prog [...] & spawned another process for the server.sh script. The processes looked like this:

USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
appuser        1  0.0  0.0   4884  3752 ?        Ss   09:47   0:00 /bin/bash ./server.sh -d
appuser        6  0.0  0.0   5020  2224 ?        S    09:47   0:00 /bin/bash ./server.sh -d
appuser        7 45.3  2.1 22729132 610944 ?     Sl   09:47   0:16 java -ea -Dcom.sun.management.jmxremote=true -Dcom.sun.management.jmxremote.local.only=false -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management

The reason I got another server.sh process is that eval $prog [...] &, first interpreted the & and forked a subshell(PID 6), then run the eval command there. This is obviously not the desired behaviour as the stored pid will be the one for the script, so the KILL signal will not be the forwarded to the the Java process.

However, I have the solution for this: escaping the ampersand & character. This way the ampersand character will be part of the eval command. So, running the command like this eval $prog [...] \& will add the ampersand inside the eval command. Therefore, the Java process will be launched in background from the context of the main server.sh script. The stored pid will be indeed the one for the Java process, which will allow to forward the KILL signal using the trap from #4369-94.

#98 Updated by Roger Borrello 6 months ago

Andrei Plugaru wrote:

However, I have the solution for this: escaping the ampersand & character. This way the ampersand character will be part of the eval command. So, running the command like this eval $prog [...] \& will add the ampersand inside the eval command. Therefore, the Java process will be launched in background from the context of the main server.sh script. The stored pid will be indeed the one for the Java process, which will allow to forward the KILL signal using the trap from #4369-94.

Very clever... and your testing confirms?

#99 Updated by Andrei Plugaru 6 months ago

Roger Borrello wrote:

Very clever... and your testing confirms?

Yes, it works as expected. The trap from server.sh catches the KILL signal and forwards it to the java process, resulting in the server being gracefully shutdown. This means that our shutdown hooks are executed.

#100 Updated by Andrei Plugaru 6 months ago

As the discussion regarding the synchronization of metadata tables was not officially closed, I have done some analysis regarding a best approach in the context of clustering.

First, regarding the so-called hybrid approach. Even though, in the non-clustered mode it may have showed better performance because of not having populate the H2 tables from scratch, in the context of cluster mode, this approach implies more data to be synchronized. More exactly the constant updates to the variables that hold the map version and the versions for the entities. In the end, the overhead of synchronizing these variables may offset the gain of avoiding the repopulation of the H2 table.

That being said, I have come up to an approach that should mainly keep the advantage of avoid constant repopulation of the H2 tables, however, in the same time, avoiding synchronizing additional structures besides the main backing structure. Basically, I am thinking to take advantage of the cache listener Geode provides. Inside the callbacks in the listener, we can add the affected key to a local "dirty set". This operation adds close to zero overhead. The actual H2 synchronization is obviously deferred until a 4GL query on the metadata table actually occurs. At that point, we iterate only the dirty keys, read their current values from the distributed map and override the entry in H2 with that value. After the sync, the dirty set is cleared.

This approach avoids full H2 table repopulation on each query while tracking updates with minimal overhead.

#101 Updated by Roger Borrello 6 months ago

Andrei Plugaru wrote:

Roger Borrello wrote:

Very clever... and your testing confirms?

Yes, it works as expected. The trap from server.sh catches the KILL signal and forwards it to the java process, resulting in the server being gracefully shutdown. This means that our shutdown hooks are executed.

Is this backward compatible with non-Docker configuration? If so, I think it should be added in. It could also be protected with a check for the /.dockerenv file, as well, to ensure it is only used in a Docker situation.

#102 Updated by Andrei Plugaru 6 months ago

Roger Borrello wrote:

Is this backward compatible with non-Docker configuration? If so, I think it should be added in. It could also be protected with a check for the /.dockerenv file, as well, to ensure it is only used in a Docker situation.

Well, yes, I left that change there in my dataset project and didn't have any issues while I was working for other tasks.

However, I didn't try so far any other mode of the server.sh script(except the debug mode :)) ). I hope there isn't anything that interferes with this new mode of starting the Java process.

On another note, as this isn't an exact component of the clustering work, I think we should create a new task where we further discuss and test this solution.

#103 Updated by Andrei Plugaru 6 months ago

  • Related to Bug #11157: Improve FWD server startup mode in server.sh added

#104 Updated by Andrei Plugaru 5 months ago

I have continued working on changes needed for clustering in FWD. Even though we initially agreed that caches(including FFCache) don't need synchronization because we thought only of performance implications. After further analysis, it actually seems the invalidations on FFCache need to be propagated throughout the cluster. Without this we risk of seeing stale data on some servers after it was only invalidated on others.

So, I went ahead and implemented the propagation of invalidation throughout the cluster. I have implemented that using Geode Functions which allow execution of code on another members of the cluster. So, at each invalidation in FFCache, that type of invalidation(e.g. invalidation of an index, invalidation of a dmo list) is distributed to the other servers. Currently, I have set this operation asynchronous which should give eventual consistency for the cache, however, it should give us better performance.

This is currently in 4369a/16379.

#105 Updated by Alexandru Lungu 5 months ago

FYI: I created https://proj.goldencode.com/projects/p2j/wiki/Stateless_Server_Clustering for documentation on this feature. Even if incomplete, we will track the decisions and configurations there.

#106 Updated by Andrei Plugaru 5 months ago

Alexandru Lungu wrote:

FYI: I created https://proj.goldencode.com/projects/p2j/wiki/Stateless_Server_Clustering for documentation on this feature. Even if incomplete, we will track the decisions and configurations there.

Got it, will start working on the documentation.

On another note, I have committed 4369a/ rev. 16380.

I have fixed here a potential recursive invalidation of the FFCache entries. Based on the fact that Geode Functions execute on some specific threads, I have added a thread local variable in FastFindCache meaning whether the thread is handling the remote invalidation requests or is just a normal FWD thread. By default the value is false, it is only set to true in the execute method from the function implementation. This way the method which distributes the FFCache invalidations to other servers executes if the source of invalidation is really this server.

#107 Updated by Roger Borrello 5 months ago

For the configuration, the ./deploy/server/prepare_dir.sh can be updated to look for a new section in the JSON, which could look something like:

  "cluster": {
    "geode_locator_host": "locator",
    "geode_locator_port": "10334" 
  }

And a directory_cluster.xml.template:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<remapper-storage>
  <node class="container" name="">
    <node class="container" name="server">
      <node class="container" name="default">
        <node class="boolean" name="cluster-mode">
          <node-attribute name="value" value="TRUE"/>
        </node>
        <node class="container" name="geode">
          <node class="string" name="locatorHost">
            <node-attribute name="value" value="{geode_locator_host}"/>
          </node>
          <node class="integer" name="locatorPort">
            <node-attribute name="value" value="{geode_locator_port}"/>
          </node>
        </node>
      </node>
    </node>
  </node>
</remapper-storage>

The prepare_dir.sh would attempt to read, and process the values if found.

# Handle cluster portion as appropriate
if $(_getval "cluster" "$file"); then
   locator_host=$(sub_getval "cluster" "geode_locator_host" "locator" $infile)
   locator_port=$(sub_getval "cluster" "geode_locator_port" "10334" $infile)
   sed \
       -e "s/{geode_locator_host}/$locator_host/g" \
       -e "s/{geode_locator_port}/$locator_port/g" \
       directory_cluster.xml.template > directory_cluster.xml
   rc=$?
   if [ $rc -ne 0 ]; then
      echo "ERROR: Could not create directory_cluster.xml rc=$rc" 
      exit $rc
   fi
   directory_copy "directory_cluster.xml" "/server" "directory_tmp.xml" "/server" 
   rm directory_cluster.xml
fi

This is pretty easy to add to Hotels as a sample. Let me know if this is to be done now.

The other end is the configuration of other things like nginx and the compose file. These could be read from the same JSON as we use to setup directory and server.xml. But I am trying to design how to handle the JSON for this purpose for a project, but a discussion here for clustering would bear the same fruit for both projects.

If we start including compose and other configuration data in the same JSON as we are currently using for the directory (see the hotel_gui ./deploy/etc/prepare_dir_pg_broker_allinone.json for a sample) things are a little cluttered and could use some better design. For example, if we intermingle the compose information with the main JSON, we get fields like ip4_address, which is used to assign an IP address to a service when macvlan is used. Same thing with swarm_node if you are configuring the Swarm Stack.

Would it be better to break out the other config items to a compose or nginx section to keep that data separate, or perhaps put them under the cluster section to group them together?

  "cluster": {
     "geode_locator_host": "locator",
     "geode_locator_port": "10334" 
     "nginx": {
        "upstream": "backend_server",
        "upstream_server": "FWD_server",
        "upstream_port": 7443
     }
  }

In this JSON sample, we could use "admin_port", which is already in the JSON instead of repeating the same data in "upstream_port". But if the value could be different, we'd need to keep it separate. This creates a "monolithic" JSON, but in the case of an applications configuration, that might not be so bad.

We could also go the route of an entire JSON dedicated to the other configurations, like an nginx.json and a compose.json, which would make configuration more granular, but also create duplicated effort and possible misconfiguration, as you have to have each JSON contain all data required. The script could pull together what it needs by pulling some config values from the application JSON and some from another JSON as needed.

Any opinions would be appreciated.

#108 Updated by Roger Borrello 5 months ago

Alexandru Lungu wrote:

FYI: I created https://proj.goldencode.com/projects/p2j/wiki/Stateless_Server_Clustering for documentation on this feature. Even if incomplete, we will track the decisions and configurations there.

Regarding graceful shutdown, shouldn't FWDLauncher shutdown handled the clean disconnect from any configured cluster, instead of trying to hook to the JVM process shutdown? At the very least, an external script to perform it cleanly should be provided so a clean shutdown of the container would consist of running the script within the container before shutting things down?

#109 Updated by Greg Shah 5 months ago

Let's discuss this in another task (create one for this). I don't want to pollute this one with devops/scripting discussions.

I don't think we are ready (yet) for this to be integrated into our standard devops tooling.

#110 Updated by Roger Borrello 5 months ago

Greg Shah wrote:

Let's discuss this in another task (create one for this). I don't want to pollute this one with devops/scripting discussions.

I don't think we are ready (yet) for this to be integrated into our standard devops tooling.

In FWD:Deployment?

#111 Updated by Greg Shah 5 months ago

Yes

#112 Updated by Alexandru Lungu 4 months ago

  • Assignee changed from Andrei Plugaru to Eduard Soltan

#113 Updated by Eduard Soltan 4 months ago

  • Related to Bug #11329: Stateless FWD server clustering setup questions added

#114 Updated by Eduard Soltan 4 months ago

I’m looking to wrap up the last remaining item for the cluster synchronization: the metadata tables.

We previously explored two potential paths: the Hybrid approach involving incremental updates via versioning, and the On-Demand approach focusing on clearing and repopulating metadata tables during queries. Has a decision taken in this regard?

#115 Updated by Eric Faulhaber 3 months ago

Eduard, we actively have been making changes to the implementation of the updatable metadata tables to overcome locking and performance issues in the H2 page store engine. The final outcome of that work will inform the clustering implementation, but the preferred implementation in that performance work is the hybrid approach. Please see references to VSTs in #11282, #11300, #11301.

#116 Updated by Eric Faulhaber 3 months ago

  • Related to Bug #11379: implement "live" metadata tables in persistent database added

#118 Updated by Eduard Soltan 3 months ago

If we disable the usage of MetadataTables from the directory configuration, and if we still encounter an usage of the a disabled Metadata table. In this case we should terminate the execution of the program entirely?

If the solution is to terminate the program, I don't think that the ErrorConditionException is the right choice here. For example case like find first _Trans no-error will just register the the error, and continue the execution which could generate some unexpected behaviour in the program execution.

#119 Updated by Ovidiu Maxiniuc 3 months ago

If the legacy code tries to use one of the _meta tables, I think it is more logical to raise a STOP condition instead.

#120 Updated by Greg Shah 3 months ago

I'm OK with a STOP condition.

#121 Updated by Eduard Soltan 3 months ago

Committed on 4369a, disabled runtime support VSTs.

Added new class MetaConfig, to keep VSTs enable/disable flags.

Added another UnsupportedMetadataException class that extends the StopConditionException, in this way we could bypass do on stop undo, leave control-of-flow statements.

Also added the following nodes in directory.xml under server/default

<node class="boolean" name="lock-table">
  <node-attribute name="value" value="FALSE"/>
</node>
<node class="boolean" name="connection-table">
  <node-attribute name="value" value="FALSE"/>
</node>
<node class="boolean" name="transaction-table">
  <node-attribute name="value" value="FALSE"/>
</node>
<node class="boolean" name="user-table-stat">
  <node-attribute name="value" value="FALSE"/>
</node>
<node class="boolean" name="tenant-table">
  <node-attribute name="value" value="FALSE"/>
</node>

#122 Updated by Constantin Asofiei 3 months ago

Eric, these settings are more under persistence, per-database, and not per server.

#123 Updated by Eduard Soltan 3 months ago

I started 2 serves with the script from #11392.

I get the following error:

Caused by: java.lang.RuntimeException: Unresolvable remote export public abstract void com.goldencode.p2j.util.osresource.FileSystem.setLegacyFileSystemParameters(java.lang.String,boolean,java.util.Map,java.lang.String[],boolean).
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.obtainRoutingKey(RemoteObject.java:1575)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1458)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:144)
    at jdk.proxy2/jdk.proxy2.$Proxy171.setLegacyFileSystemParameters(Unknown Source)
    at com.goldencode.p2j.util.FileSystemOps$ContextContainer.initialValue(FileSystemOps.java:1754)
    at com.goldencode.p2j.util.FileSystemOps$ContextContainer.initialValue(FileSystemOps.java:1700)
    at com.goldencode.p2j.security.ContextLocal.getImpl(ContextLocal.java:556)
    at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:460)
    at com.goldencode.p2j.util.FileSystemOps$ContextContainer.obtain(FileSystemOps.java:1710)
    at com.goldencode.p2j.util.FileSystemOps.getProperty(FileSystemOps.java:805)
    at com.goldencode.p2j.util.FileSystemOps.getProperty(FileSystemOps.java:788)
    at com.project.openedgecode.gui_code.Utility.lambda$getEnvironmentValue$171(Csdutility.java:2362)
    at com.goldencode.p2j.util.Block.body(Block.java:636)

In ServerResourceManager.initializeFileSystem the following code is hit:

if (isServerSide(OsResourceType.FILESYSTEM))
{
  System.loadLibrary("p2j"); // required by working-dir and others

  boolean allServer = isAllServerSide();
  fs = FileSystemDaemon.initializeServer(allServer);
}
else
{
  fs = (FileSystem) RemoteObject.obtainInstance(FileSystem.class, true);
}

FileSystem is obtained from RemoteObject

#124 Updated by Constantin Asofiei 3 months ago

Eduard, please post your directory.xml.

#125 Updated by Eduard Soltan 3 months ago

  • File directory.xml added

Is it about server-side-resources node? I didn't see it set in any project.

#126 Updated by Constantin Asofiei 3 months ago

Eduard Soltan wrote:

Is it about server-side-resources node? I didn't see it set in any project.

Please post the full log where you get the exception. It doesn't make sense for this to happen.

#127 Updated by Eduard Soltan 3 months ago

[Deleted]

#128 Updated by Eduard Soltan 3 months ago

  • File deleted (directory.xml)

#130 Updated by Eduard Soltan 3 months ago

A little update on running a stateless cluster on a customer application.

This stack trace shows a heavy lock contention during region creation, caused by the Geode Management layer trying to keep up with your high region count primarily from UniqueTracker. To monitor these stats, it calls StatMonitorHandler.addMonitor(). This method requires a global lock on the StatMonitorHandler object.

I disabled the statistics on each node during connection:

Properties props = new Properties();
props.setProperty("name", nodeName);
props.setProperty("locators", getLocatorString());
props.setProperty("jmx-manager-update-rate", "60000");
props.setProperty("jmx-manager-start", "false");

props.setProperty("statistic-sampling-enabled", "false");
props.setProperty("enable-time-statistics", "false");

return new CacheFactory(props).create();

It does not affect the data distribution between nodes (redundancy and consistency), but it reduce cluster statistics quality. Now the script never halts, and time execution is comparable to one node without geode configuration.

However the core issue here is the pattern that we use in UniqueTracker. We create 2 separate Regions for every table. This becomes quite unbearable for geode to monitor. I think we can reduce that to just 2 Region per node and have the table as a key.

In Apache Geode, a Region is not equivalent to a simple HashMap or a row in a database; it is functionally equivalent to a database table.

When we dynamically create regions (2 regions for each unique index), Geode has to allocate a significant amount of overhead for each region:

- Memory Overhead: Each region maintains its own concurrent data structures, statistics monitors, and metadata.
- Thread Overhead: Regions often spin up dedicated threads for expiration, eviction, and asynchronous event queues.
- Cluster Coordination: Creating a region is a cluster-wide operation. Doing this dynamically at runtime forces the cluster configuration service to lock and synchronize state across all nodes, which is slow and can cause distributed deadlocks or latency spikes under load.

Geode is optimized for a relatively static, low number of regions (usually in the tens or hundreds).

What I think could be done we order to solve this:

Have a 2 Global Regions: records and entries.

Instead of of doing this per index:

Region: IndexA_records -> Key: 123, Value: Data

Region: IndexB_records -> Key: 123, Value: Data

We can change to something like this:

Region: Global_Index_Records -> Key: IndexA_123, Value: Data

Region: Global_Index_Records -> Key: IndexB_123, Value: Data

Since the Key for records and entries maps are not simple Strings they should be adjusted for distributed mode.

#131 Updated by Constantin Asofiei 3 months ago

Some other notes:
  • we need to check all nodes use the same JDBC configurations (URL, passwords, etc), for landlord and connected databases
  • we need to determine how to sync directory.xml runtime changes via REST API (or admin console) across the cluster.
  • any other resources the FWD server can change and must be synced across the cluster?

#132 Updated by Greg Shah 3 months ago

As mentioned on the call this morning, I think that in a clustered environment we need a centralized directory. My initial idea is that this would be independent of Geode. Our early directory work already had an option to implement LDAP as a back end (our early LDAP code proabbly doesn't work but it still exists) and since that time there are many new options available. We also had a concept that in a cluster, one server would coordinate the edits (a master directory server), but I don't think we implemented that.

We have the Remapper interface that is specifically designed to hide the back-end implementation. I think we should implement a centralized directory back-end that can be shared across a cluster. See our old documentation for some useful details on the original design.

If we do it this way, I think it will scale better than if we build our own synching using Geode. This centralized use case is also something that is well supported by technologies lik Redis.

We can still consider getting LDAP going but LDAP brings an extra level of work. In fact our early work to support both XML and LDAP back-ends really overcomplicated the directory design on the XML side. It was already implemented before I realized that. This is why we have tasks #2316 and #1842. My point here is that we intend to simplify our core directory design to be less verbose on the XML side. I don't know how that might also allow simplification of the external APIs and Remapper interface.

My instinct says we should keep this back-end simple (not LDAP) and fast. But we should discuss it. Our customer may have a preference for something like LDAP which will have a robust infrastructure and tooling surrounding it.

#133 Updated by Eduard Soltan 3 months ago

I little bit about my work on implementation of stateless clustering:

- During some experiments I determined that GLOBAL type for lockTable is redundant. When performing any operation of this kind of region Geode will try to acquire the global distributed lock for the specific key it tries to modify.

And with the design we have for LockManger, this type of region is especially prone to dead locks. We have a distribute lock for the entire tenant table (lockForLockTable), and one node has successfully acquired that lock. At this exact moment, all other nodes that try to acquired the same lock get block at lockForLockTable.lock. Now the node that already helds lockForLockTable, tries to acquire the lock for the specific key it tries to modify. It results in a deadlock.

So I had to change the region type to DISTRIBUTED_ACK, were the change operation are performed without the global lock acquisition. And this is not required since we have our custom synchronization mechanism.

- some proper locking mechanism was not implemented in DistributedInMemoryLockManager.lockWhenAlreadyHeld and waitUntilStatusIsModified, the old synchronize logic was removed but distributed lock was not applied.

- changed lockers in LockStatusImpl from IdentityHashMap to HashMap in order to use overridden methods equla and hashCode.

- added a check for distributed node availability if it is holding a lock.

- for UniqueTracker I reduced the number of regions, now it uses just 2 region. And UniqueIndex has a new API for changes on records and entries maps (please see #4369-130).

With this changes I tested pretty heavily on a environment of 100 tenants and could not spot any deadlocks.

#134 Updated by Eduard Soltan 3 months ago

About the need to synchronize directory changes, I think we need to implement a centralized directory back-end now (probably using Redis) since it should be simple to implement and pretty fast when multiple directory writes are happening. And if the customer will require it, the finish support for LDAP.

There is a locking mechanism in DirectoryService, I think Geode locking mechanism could still be used it we run in cluster mode.

#135 Updated by Greg Shah 3 months ago

Yes, we need to implement the directory solution now.

I'm OK in principle with a redis based backend. I assume that we can implement the exclusive locking semantics properly from the remapper code.

I think Geode locking mechanism could still be used it we run in cluster mode.

If the directory is centralized, why use Geode for locking here?

#136 Updated by Eduard Soltan 3 months ago

Greg Shah wrote:

Yes, we need to implement the directory solution now.

I'm OK in principle with a redis based backend. I assume that we can implement the exclusive locking semantics properly from the remapper code.

Yes, we could use RedissonClient to handle critical section in remapper.

If the directory is centralized, why use Geode for locking here?

I was thinking about LockManager in DirectoryService. But I have to more into it.

Question why does values in Attribute class is an Vector? Shouldn't it be just a simple Object?

#137 Updated by Constantin Asofiei 3 months ago

Eduard Soltan wrote:

Question why does values in Attribute class is an Vector? Shouldn't it be just a simple Object?

See NodeAttribute.multiple, which is set from multiple attr in dir schema <class-attribute.

#138 Updated by Eduard Soltan 3 months ago

There is something that Constantin mentioned in the last stand up.

Initially, I load the data to Redis by parsing the old directory in the xml format and creating the structure in redis. To avoid doing the same for later connected nodes, and to persist the changes made by the application runtime, I think that we could keep the separate key in Redis that will tell if the directory was initialized. To keep the data safe in redis, it can be configured to take snapshots of its state more eagerly.

#139 Updated by Eduard Soltan 3 months ago

Committed 4369a, rev. 16534.

Added RedisRemapper.java

This class implements the Remapper.java interface. We model the hierarchical structure, using prefix-based key schema that breaks each node down into three distinct components.

- First, a node's core existence and object type are stored as a simple string under the key dir:class:{nodeId}
- Second, parent-child relationships are maintained using Redis Sets located at dir:children:{parentId}
- Third, the node's custom properties—are serialized and stored individually under precise keys formatted as dir:attr:{nodeId}:{attributeName}

Added XmlRedisRemapperIO.java

This class handles Redis directory initialization. Parses the old xml directory and populate the Redis database. We have a special key (init), if this key is set the directory is not re-initialized.

Added RedisLockManager.java

This is a implementation of LockManager, which I changed to a interface. Initially I implemented the directory changes implementation in RedisRemapper, but I think in this way we lose not trivial locking mechanism from LockManager.

To obtain the same level of synchronization as in old lock manager (RO, RW, RX, WX), I have to orchestrate 2 @RReadWriteLock@s.

#140 Updated by Constantin Asofiei 3 months ago

Greg, about persisting the changes to disk: what is the approach here? I would think that a node gets a signal from redis (or checks redis on interval) to write the directory.xml to disk?

#141 Updated by Eduard Soltan 3 months ago

Constantin Asofiei wrote:

Greg, about persisting the changes to disk: what is the approach here? I would think that a node gets a signal from redis (or checks redis on interval) to write the directory.xml to disk?

But isn't this something that Redis does internally, I mean it does persist its state and the next time we just connect to an instance of Redis and have all the changes there? No need to initialize its state again.

#142 Updated by Constantin Asofiei 3 months ago

I think there's more just about having it persistent in redis. Lets assume a maintenance window where the entire cluster gets shutdown for an upgrade, which requires also some directory.xml changes: how do we get this into redis if all we have to change is directory.xml, which is obsolete and not in sync with the changes made while the cluster were up, as they exist only in redis?

#143 Updated by Eduard Soltan 2 months ago

Constantin Asofiei wrote:

I think there's more just about having it persistent in redis. Lets assume a maintenance window where the entire cluster gets shutdown for an upgrade, which requires also some directory.xml changes: how do we get this into redis if all we have to change is directory.xml, which is obsolete and not in sync with the changes made while the cluster were up, as they exist only in redis?

Maybe we should have a tool that export the redis data into xml format, and unset init key (such that at the next startup the redis directory will be updated).

#144 Updated by Constantin Asofiei 2 months ago

Eduard Soltan wrote:

Constantin Asofiei wrote:

I think there's more just about having it persistent in redis. Lets assume a maintenance window where the entire cluster gets shutdown for an upgrade, which requires also some directory.xml changes: how do we get this into redis if all we have to change is directory.xml, which is obsolete and not in sync with the changes made while the cluster were up, as they exist only in redis?

Maybe we should have a tool that export the redis data into xml format, and unset init key (such that at the next startup the redis directory will be updated).

Maybe a REST API, really secured? Greg?

#145 Updated by Eduard Soltan 2 months ago

Eduard Soltan wrote:

Added RedisLockManager.java

This is a implementation of LockManager, which I changed to a interface. Initially I implemented the directory changes implementation in RedisRemapper, but I think in this way we lose not trivial locking mechanism from LockManager.

To obtain the same level of synchronization as in old lock manager (RO, RW, RX, WX), I have to orchestrate 2 @RReadWriteLock@s.

I tested LockManagerTest with the RedisLockManager implementation. 4369a, rev. 16535 solves all the deadlocks.

#146 Updated by Greg Shah 2 months ago

A clustered environment should have a single directory that contains the configuration for all servers. That directory will be hosted in Redis and each server will connect to it using the RedisRemapper.

We need to discuss the format in which this shared directory is stored, how we read this into Redis and how we save changes to it.

  • Do we provide an import/export process while the directory itself is stored in some Redis-native format?
  • Do we auto-import from the first server that starts? If so we would need to auto-export when the last server stops.
  • Some other approach?

We also need to discuss if there would be some kind of active/active replica also there so that the directory is not a single point of failure.

#147 Updated by Greg Shah 2 months ago

Eduard Soltan wrote:

Eduard Soltan wrote:

Added RedisLockManager.java

This is a implementation of LockManager, which I changed to a interface. Initially I implemented the directory changes implementation in RedisRemapper, but I think in this way we lose not trivial locking mechanism from LockManager.

To obtain the same level of synchronization as in old lock manager (RO, RW, RX, WX), I have to orchestrate 2 @RReadWriteLock@s.

I tested LockManagerTest with the RedisLockManager implementation. 4369a, rev. 16535 solves all the deadlocks.

Are you talking about the database locking here?

#148 Updated by Eduard Soltan 2 months ago

Greg Shah wrote:

Are you talking about the database locking here?

No, it is about com.goldencode.p2j.directory.LockManager lock manager for DirectoryService.

#149 Updated by Eduard Soltan 2 months ago

Greg Shah wrote:

  • Do we provide an import/export process while the directory itself is stored in some Redis-native format?

No, as of right now. We do support import/export from xml format to redis.

  • Do we auto-import from the first server that starts? If so we would need to auto-export when the last server stops.

Yes, we have a special key stored in redis (init), if it is not set then the data is auto-imported. If it is set then the data is not auto-imported.

#150 Updated by Greg Shah 2 months ago

Yes, but should it work that way? And we certainly would need to have an auto-export as well or the changes would be in danger of not being persisted.

#151 Updated by Eduard Soltan 2 months ago

Greg Shah wrote:

Yes, but should it work that way? And we certainly would need to have an auto-export as well or the changes would be in danger of not being persisted.

Sure, and I assume that the first node in the cluster should always auto-import the data into redis?

#152 Updated by Constantin Asofiei 2 months ago

I would go with 'some other approach' to not get too complicated when maintaining a live system. Each FWD node is responsible to have its own copy of the directory persisted to disk, as soon as some other node had made changes in redis. Have a dedicated thread listening for this event and persist it to disk immediately (in other words, when a change is made in redis, all nodes are notified to persist to disk).

In case of maintenance, you can:
  • stop a single node, and you are sure its directory is the latest one
  • upgrade this node or make edits to the directory
  • restart this node and force re-loading the directory into redis
  • other nodes after that can be upgraded and restarted without re-loading the directory in redis
  • the cluster is live and can serve requests all this time
The alternative would be separate maintenance, which includes:
  • run the tool to export the redis directory to disk
  • make edits to the directory
  • load it into redis
  • restart the nodes in the cluster to use the new directory info

#153 Updated by Eduard Soltan 2 months ago

I have been testing the changes on cust001.

I observed a pretty slow initialization at startup because of the following configuration:

<node class="container" name="tempClient">
    <node class="integer" name="poolSize">
         <node-attribute name="value" value="100"/>
    </node>
</node>

The most time consuming method is SecurityCache.readUsers where we do the following:

 isEnabled   = ds.getNodeBoolean(fullId, "enabled");
         isProtected = ds.getNodeBoolean(fullId, "protected");
         person   = ds.getNodeString(fullId,    "person");
         alias    = ds.getNodeString(fullId,    "alias");
         password = ds.getNodeByteArray(fullId, "password");
         cdate    = ds.getNodeDate(fullId, "pwsetdate");
         ctime    = ds.getNodeTime(fullId, "pwsettime");
         groupIds = ds.getNodeStrings(fullId,   "groups");
         mode     = ds.getNodeInteger(fullId,   "mode");
         authPlugin = ds.getNodeString(fullId, "auth-plugin");
         osUser   = ds.getNodeString(fullId, "osuser");
         webServiceToken = ds.getNodeString(fullId, "webServiceToken");
         email    = ds.getNodeString(fullId, "email");
         oidcUuid = ds.getNodeString(fullId, "oidcUuid");

For every attribute we have to make a round trip to redis. Maybe it can be optimized to get the whole class at once with all the attributes?

#154 Updated by Eduard Soltan 2 months ago

This are DirectoryService (RedisRemapper) recorded calls in order to create just one temp client (please notices that this is just a part of the queries):

Average time taken to create 1 temp client with RedisRemapper is comparable to the time taken to create 100 clients with RamRemapper. I do notices an empiric thing that if we retrive an atribute from a class, it is very likely that we would retrive all attributes in the same class. So maybe we could use this info in order to cache some attibues in a class and reduce the number of Redis queries.

#155 Updated by Constantin Asofiei 2 months ago

Greg, our current approach for the 'temp users' to authenticate NativeSecureConnection calls can be improved:
  • currently, any FWD user can be used (as long as you send the right user/password), not just 'temp users'
  • in a cluster mode, these 'temp users' need to be private per-node
  • I think we need to move these 'temp users' in memory (outside of directory), and force the spawner to work only with these users

#156 Updated by Eduard Soltan 2 months ago

Constantin Asofiei wrote:

Greg, our current approach for the 'temp users' to authenticate NativeSecureConnection calls can be improved:
  • currently, any FWD user can be used (as long as you send the right user/password), not just 'temp users'
  • in a cluster mode, these 'temp users' need to be private per-node

I guess in this case deleteTemporaryAccounts don't not make sense anymore. The user creation should go through all the steps from SecurityAdmin.addUser, but the actual node in the directory should not be inserted and SecurityCache not updated (only the user/group array). After all temp user creation, security cache should be updated.

  • I think we need to move these 'temp users' in memory (outside of directory), and force the spawner to work only with these users

Don't spawner work only with temp users now. ClientSpwner.spawn calls TemporaryAccount.open() which gets a user from temp users pool?

#157 Updated by Constantin Asofiei 2 months ago

Eduard Soltan wrote:

Don't spawner work only with temp users now. ClientSpwner.spawn calls TemporaryAccount.open() which gets a user from temp users pool?

I don't mean when you call spawn binary from FWD. I mean if you do something like this:

P2J_SUBJECT=61646D696E P2J_PASSWORD=74657374313233 ./spawn 0 3334 localhost standard 07600757-cafe-4f32-ba65-15dce27829c4

it will authenticate (here subject/pass is admin/test123).

#158 Updated by Eduard Soltan 2 months ago

Committed on 4369a, rev. 16539. Keep the temp users in memory , and imporved support for NativeSecureConnection. Creation of 100 temp users on hotel is definitely much faster, have to test it on customer app.

#159 Updated by Constantin Asofiei 2 months ago

Eduard Soltan wrote:

Committed on 4369a, rev. 16539. Keep the temp users in memory , and imporved support for NativeSecureConnection. Creation of 100 temp users on hotel is definitely much faster, have to test it on customer app.

On a first glance, I think is OK. An additional change would be to not have a limited pool of in-memory temp-clients, but allow it to expand as needed. But, on a second thought, these users can be 'one time use', create it before spawn and delete it once it is used. If we bypass directory changes, this should not affect performance.

Otherwise, please compare with the trunk rev and start cleaning it up (javadocs, headers, copyright year, formatting, etc).

#160 Updated by Eduard Soltan 2 months ago

Constantin Asofiei wrote:

I would go with 'some other approach' to not get too complicated when maintaining a live system. Each FWD node is responsible to have its own copy of the directory persisted to disk, as soon as some other node had made changes in redis. Have a dedicated thread listening for this event and persist it to disk immediately (in other words, when a change is made in redis, all nodes are notified to persist to disk).

I am a bit worried about performance impact of this one. Sure it execute on a separate thread, but Redis is single threaded by nature and in this case some real application request will have to wait in line for some maintenance request from all the cluster nodes to finish. And this also seems superfluous, as redis already does its backup regularly.

In case of maintenance, you can:
  • stop a single node, and you are sure its directory is the latest one

What if in mean time some other node has made a change in Redis directory, in this case it is not the latest one.

The alternative would be separate maintenance, which includes:
  • run the tool to export the redis directory to disk
  • make edits to the directory
  • load it into redis
  • restart the nodes in the cluster to use the new directory info

It seem to me that this is the best option.

#161 Updated by Greg Shah 2 months ago

Constantin Asofiei wrote:

I would go with 'some other approach' to not get too complicated when maintaining a live system. Each FWD node is responsible to have its own copy of the directory persisted to disk, as soon as some other node had made changes in redis. Have a dedicated thread listening for this event and persist it to disk immediately (in other words, when a change is made in redis, all nodes are notified to persist to disk).

This is reasonable. The biggest concern of mine is that when you have multiple copies of the directory there may be a chance of the directory files becoming out of sync (different contents). What can we do to ensure that never happens?

In case of maintenance, you can:
  • stop a single node, and you are sure its directory is the latest one
  • upgrade this node or make edits to the directory
  • restart this node and force re-loading the directory into redis
  • other nodes after that can be upgraded and restarted without re-loading the directory in redis
  • the cluster is live and can serve requests all this time

This is good.

The alternative would be separate maintenance, which includes:
  • run the tool to export the redis directory to disk
  • make edits to the directory
  • load it into redis
  • restart the nodes in the cluster to use the new directory info

This has too much friction to be workable.

#162 Updated by Greg Shah 2 months ago

In case of maintenance, you can:

  • stop a single node, and you are sure its directory is the latest one

What if in mean time some other node has made a change in Redis directory, in this case it is not the latest one.

We need a way to lock the system from updates while there is some offline process editing the "master source".

#163 Updated by Eduard Soltan 2 months ago

Greg Shah wrote:

We also need to discuss if there would be some kind of active/active replica also there so that the directory is not a single point of failure.

I think for this one we should configure the Redis Sentinel pool. We should have a Master node, and a few Replica nodes. Sentinel Nodes that monitor the health of Master and Replica nodes. In case of a failover sentinel nodes decides which nodes should take over.

But that is totally dependent on how we are going to deploy Redis, most cloud provides for example do all this failover/replication architecture under the hood.

#164 Updated by Eduard Soltan 2 months ago

Committed on 4369a, rev. 16544.

This implements the RedisSynchronizer, a thread that listen for redis updates using a RedisStream. RediStream ensure that the directory changes are always persisted, and reaches its listener. Once RedisSynchronizer has received a directory change it, saves the directory on the disk. This ensures that the directories does not get out of sync.

The way I see how the maintenance should be performed: first set a maintenance key in Redis, this will stop all other nodes from changes to the directory.

Gracefully stop the node, I have added a shutdown hook that allows RedisSynchronizer thread to finish the write to disk.

Also the changes contain the elimination to tempUsersPool, now the TemporaryAccount is created only by the ClientSpawner.spawn.

#165 Updated by Greg Shah 2 months ago

I think we need to add an incrementing 64-bit integer revision number to the directory. This can be used to detect if any directory is out of sync.

#166 Updated by Eduard Soltan 2 months ago

Committed on 4369a, rev. 16549.

Added directory versioning. Improved support temp user creation.
General code clean up, added history entries and java docs.

#167 Updated by Eduard Soltan 2 months ago

Committed on 4369a, rev. 16552.

Support for Redis directory and lockManager backend (for directory/database). Please review.

I also worked on refactor of the directory lock manager, that employees lua scripts (pl/sql for redis). Some empiric results shows that it faster (client startup), but testing on LockManagerTest is a bit slower. So I have to lock more in depth.

#168 Updated by Constantin Asofiei 2 months ago

Eduard, please rebase 4369a.

#169 Updated by Eduard Soltan 2 months ago

Rebased to trunk 16570.

#170 Updated by Eduard Soltan 2 months ago

Committed on 4369a, rev. 16608. Multiple code fixup, that appeared during application debug.

#171 Updated by Constantin Asofiei 2 months ago

Eduard Soltan wrote:

Committed on 4369a, rev. 16608. Multiple code fixup, that appeared during application debug.

I think you need to push the branch and bind it - I don't see rev 16608.

#172 Updated by Eduard Soltan 2 months ago

Constantin Asofiei wrote:

Eduard Soltan wrote:

Committed on 4369a, rev. 16608. Multiple code fixup, that appeared during application debug.

I think you need to push the branch and bind it - I don't see rev 16608.

Pushed revision 16610 to devsrv.

#173 Updated by Eduard Soltan 2 months ago

I regression tested the changes on:

-chui regression testing (had to eliminate spring-beans-5.3.20.jar, spring-core-5.3.20.jar, spring-jcl-5.3.20.jar, spring-web-5.3.20.jar dependencies).

- etf (on spring project that connects to the server had to change to p2j, because I have some changes in protocol. Revert p2j to a older version, make the necessary changes in SM.authenticateClientWorker. And compile it with java 8, and update the archive).

- Large GUI application unit tests.

#174 Updated by Constantin Asofiei 2 months ago

Eduard Soltan wrote:

I regression tested the changes on:

-chui regression testing (had to eliminate spring-beans-5.3.20.jar, spring-core-5.3.20.jar, spring-jcl-5.3.20.jar, spring-web-5.3.20.jar dependencies).

This needs to be understood and fixed.

- etf (on spring project that connects to the server had to change to p2j, because I have some changes in protocol. Revert p2j to a older version, make the necessary changes in SM.authenticateClientWorker. And compile it with java 8, and update the archive).

What protocol changes exactly?

#175 Updated by Eduard Soltan 2 months ago

Constantin Asofiei wrote:

- etf (on spring project that connects to the server had to change to p2j, because I have some changes in protocol. Revert p2j to a older version, make the necessary changes in SM.authenticateClientWorker. And compile it with java 8, and update the archive).

What protocol changes exactly?

=== modified file 'src/com/goldencode/p2j/security/SecurityManager.java'
--- old/src/com/goldencode/p2j/security/SecurityManager.java    2026-04-29 13:54:04 +0000
+++ new/src/com/goldencode/p2j/security/SecurityManager.java    2026-05-20 11:40:51 +0000
@@ -1565,6 +1572,8 @@
                if (authdl > 0)
                {
                   socket.writeBytes(authData);
+                  Boolean tempUser = config.getBoolean("access", "user", "temp", false);
+                  socket.writeBoolean(tempUser);
                }
                socket.flush();
             }

In SM.authenticateClientWorker in order to keep temporary clients separately, and avoid reloading SecurityCache every time a new temp user is created.

#176 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

-chui regression testing (had to eliminate spring-beans-5.3.20.jar, spring-core-5.3.20.jar, spring-jcl-5.3.20.jar, spring-web-5.3.20.jar dependencies).

This needs to be understood and fixed.

We are using jcl-over-slf4j library. Now the 4369a changes brings in the geode-management which in turn brings the spring-jcl library. These creates a conflict between these 2 libraries, and the race is won by spring-jcl. This creates a problem with axiom, because spring-jcl is incompatible with axiom.

Fix is to exclude spring-jcl dependency.

#177 Updated by Ovidiu Maxiniuc about 2 months ago

Review of 4369a / r16611 (with focus on persistence)

This is a huge changeset. I focused on persistence mainly, but offered a hand-of-help in other locations. Most of them are related to coding aspect. The only issues I could spot my looking at the changes in meld are related to synchronisation (initialisation and access to static members). The most important is in UniqueTracker.java, in bold.

  • AdminServerImpl.java
    • line 5651: typo in method's name: pendingDeleteTempeUser instead of pendingDeleteTempUser;
    • multiple places: the name/description of @param and @return javadoc tags not properly indented;
  • MetaConfig, StopConditionException.java, GroupDef.java, TemporaryAccount.java, SessionManager.java, UserTableStatUpdater.java, ClusterConfig.java, DefaultLoginPanel.java, TrustedClientPlugin.java, Authenticator.java, SessionToken.java:
    • line 5: copyright year invalid or not updated;
  • Attribute.java
    • line 5: copyright year not updated;
    • lines 89-92: the individual classes import should be replaced with package import;
    • line 127: missing javadoc for the default constructor;
    • line 193: missing @param and @return javadoc tags;
  • Utils.java, AttributeDefinition.java, ObjectClass.java, LockType.java, ConnectTableUpdater.java, LockTableUpdater.java, TenantTableUpdater.java, TransactionTableUpdater.java:
    • should be reverted, the changes are irrelevant;
  • DirectoryService.java
    • line 5: copyright year not updated;
    • line 2073: parameter should be aligned or better joined with previous line;
  • LockManager.java
    • line 10: typo: am;
  • GuiWebSocket.java, ClientCore.java, StandardServer.java, LockManagerTest.java, Conversation.java, DefaultDirtyShareManager.java, LegacyWebSecurityManager.java, MultiSessionAppserverSecurityManager.java, ProcessAccount.java:
    • missing H record (possibly (c) year as well).
  • RamRemapper.java:
    • line 90: extends MasterRemapper should be moved (back) to its own line;
    • line 102: missing javadoc for c'tor;
    • line 104: can be dropped, the super() c'tor is implicitly called by compiler;
    • line 120: 'then' blocks should be nested in { / }, even if it is single statement;
    • line 120: the root gets initialized ro a different value is the parameter is null: this may be a bit confusing and should be at least documented in javadoc;
  • ConversionPool.java:
    • line 124: it is recommended to make localMutex final (LOG as well).
    • line 145: I am not sure if the synchronization is necessary. The ConversionPool.initialize() is invoked only once from StandardServer.bootstrap(). Are there other calls which qould require synch access? And if this happens, instance will be already initialzed and IllegalStateException will be thrown. The reason the method was synchronized was the isInitialized() getter, but now, these are unrelated (different lock objects);
  • DatabaseManager.java:
    • missing H record;
    • line 994: missing javadoc. This method is used from a single place and can be inlined.
  • FastFindCache.java
    • missing H record;
    • lines 177, 179, 763: missing javadocs;
    • line 195: question: non-permanent databases can be distributed? I think NOT. Therefore, this should be checked here and broadcast() should depend only on distributedMode;
  • GeodeConfig.java
    • line 75: import not necessary. Please also compact all import, without empty lines;
    • lines 162/163: identifiers no longer valid: DEFAULT_LOCATOR_HOST and DEFAULT_LOCATOR_PORT. Maybe with _1 / _2?
  • GeodeService.java
    • line 5: copyright year not updated;
    • line 82: missing javadoc;
    • line 102: missing some kind of synchronisation?
  • InMemoryLockManager.java
    • line 5: copyright year not updated;
    • line 344: move headlessOverride on own line; indent all parameters on same column (also for next two methods);
    • line 908: missing javadoc;
    • line 1039: indent all parameters on same column;
    • line 2111: extends should be on next line;
  • UniqueTracker.java
    • line 94: import not necessary;
    • line 193: no space after for keyword;
    • line 278: line too long. However, removing the final modifier (the methods of anonymous class are implicitly final since the anonymous class cannot be extended anymore) will reduce the length in limits;
    • line 1216: non-standard closing javadoc;
    • line 1263: implements should be on separate line;
    • line 1313: I think there is a problem with uniqueId being altered, after the hash is precomputed. Normally, it should be immutable since it is a lookup key.
    • line 1467: extends should be on separate line;
    • line 1528: typo: both articles used: the a;
  • DistributedUniqueIndex.java
    • line 79/80: extends / implements should take their own line;
    • lines 83, 86, 91, 96 : can be declared final (eventually in UPPER_CASE for constants);
    • line 93: extra *. It should go on next line with the text, in fact;
    • lines: 136, 152, etc: @Override annotation is recommended;
  • Account.java
    • tempUserId can be final (also subjectId, description and alias);
  • SecurityAdmin.java
    • lines 551, 552, 555: wrapping in a new String is not necessary;
    • lines 554-556 do not make sense. It should be simplified to: gd.authPlugin = ga.getAuthPlugin();
    • line 2330: parameter ext is not actually used. The only place I could track back it is null;
  • SecurityCache.java
    • lines 256, 267: new members can be final;
    • line 404: tempGroups is static, some kind of synchronization is necessary;
    • line 2013: tempGroups should be accessed directly, method getTempGroup exposes tempGroups unnecessarily and should be dropped;
    • line 463: public method exposes tempUsers. Should be at most package protected, not public. Better drop it completely and create a getUser(String) instead to be used from SecurityAdmin.pendingDeleteTempUser();
    • line 811: { / } are mandatory, even for single-statements blocks;
    • line 830: missing description for ae parameter (please fix);
    • line 1105/1108: duplicate line;
    • line 2021: groups is definitely not null here (see if at 2014);
    • line 2039: mode is definitely not null here (it is assigned to group.mode which is in int). It should be inlined, as the other parameters for GroupAccount;
    • line 2480: users cannot be null because of line 2455;
    • line 2483: missing { / };
    • line 2502: webServiceToken is always set to null;
    • line 2505: isEnabled cannot be null (userDef.enabled is a boolean);
    • line 2510: similar for isProtected, these should be inlined;
    • line 2571: mode@ is definitely not null here (as above);
    • line 2578: acc instanceof GroupAccount implies acc != null;
    • line 2620: oidcUuid is always null (not altered since declaration);
  • SecurityContext.java;
    • line 5: (c) year to be updated;
    • line 79: missing H entry;
    • line 187: missing javadoc;
    • lines 227, 303: missing @param for tempUser;
    • line 538: missing javadoc;
  • SecurityManager.java
    • line 835: not sure what problem the localMutex tries to fix. This will synchronize only the creation of securityManager, but not the other accesses;
    • line 4787: missing @param for tempUser;
    • line 5306: missing javadoc;
    • line 6896: missing javadoc;
    • line 7002: missing javadoc;
    • line 7026: unnecessary return statement;
  • ConfigItem.java:
    • line 27: missing H entry;
    • line 620: missing javadoc;
  • RedisConfig
    • missing standard file header;
    • line 113: missing javadoc.

#178 Updated by Constantin Asofiei about 2 months ago

Eduard, how's the testing going for other apps?

Fix is to exclude spring-jcl dependency.

I don't see this in build.gradle

Also, we will have conflicts with the app for which clustering is needed, as it uses these jars (don't mind that for some reason two versions are in the docker container):

spring-core-6.2.17.jar
spring-core-6.2.18.jar
spring-beans-6.2.17.jar
spring-beans-6.2.18.jar
spring-jcl-6.2.17.jar
spring-jcl-6.2.18.jar
spring-web-6.2.18.jar

and 4369a brings these jars:
spring-beans-5.3.20.jar
spring-core-5.3.20.jar
spring-jcl-5.3.20.jar
spring-web-5.3.20.jar

We need to move to 6.2.18 version in build.gradle - is this possible?

I'll review it completely today.

#179 Updated by Constantin Asofiei about 2 months ago

Eduard, this is review for 4369a rev 16611:
  • for the management of temp group and users
    • there should be a single, global, in-memory, temp group where the temp users are added. So the GroupDef.orderId is not required at all, SecurityCache.getTempGroup must not return a map but a GroupDef, etc
    • for the temporary user accounts, create a sub-class of UserDef and UserAccount. So the fields added to Account, UserAccount, UserDef must be in their own sub-class. The same for GroupAccount. And ProcessAccount.java and other changes no longer needed.
    • src/com/goldencode/p2j/security/SecurityAdmin.java - only a single temp group needs to exist.
    • src/com/goldencode/p2j/security/SecurityCache.java - there is no synchronization for tempGroups and tempUsers maps (tempGroups should be just a group not a map).
    • src/com/goldencode/p2j/security/SecurityContext.java - review it completely, history/copyright/formatting/javadoc
    • src/com/goldencode/p2j/security/SecurityManager.java - add a single method to resolve an account (temp or not) instead of doing ctx.isTemporary() ? in lots of places.
    • javadoc for runCustomServerHook, PasswordChange.tempUser, etc (review it fully)
  • javadoc or formatting problems
    • src/com/goldencode/p2j/directory/Attribute.java - default c'tor - please format it properly and add javadoc
    • src/com/goldencode/p2j/directory/RedisConfig.java
    • src/com/goldencode/p2j/main/TemporaryAccount.java - close - there is a commented code
  • src/com/goldencode/p2j/directory/DirectoryService.java
    • the synchronized from closeBatch(boolean, boolean) method was removed - why? closeBatch(boolean) method is still synchronized.
  • src/com/goldencode/p2j/directory/RamRemapper.java
    • extends needs to be on its own line
    • javadoc missing for constructor
  • src/com/goldencode/p2j/net/Conversation.java
    • I assume this is for the call from NativeSecureConnection? Then why not add a SessionListener when authenticating with a temp-user, to automatically delete it when session ends?
  • src/com/goldencode/p2j/util/TransactionManager.java
    • please fix the In order to use _trans table in need to enable it in directory.xml log message
  • src/com/goldencode/p2j/security/Authenticator.java
    • leave the trunk version of serverAuthHook as is and add a default serverAuthHook interface method with the tempUser overload, which just calls serverAuthHook trunk version; this way there is no need to change all the other implementations (plus we will need to change the hook at least for ChUI). And most of the changes in p2j/security package can be reverted.
  • please use star imports in:
    src/com/goldencode/p2j/directory/Attribute.java
  • missing history entries or copyright year update or entire license text or problems in Module, etc; please review each file.
    src/com/goldencode/p2j/admin/GroupDef.java
src/com/goldencode/p2j/directory/Attribute.java
src/com/goldencode/p2j/directory/DirectoryService.java
src/com/goldencode/p2j/directory/LocalLockManager.java
src/com/goldencode/p2j/main/ClientCore.java
src/com/goldencode/p2j/main/CreateAccountTask.java
src/com/goldencode/p2j/main/StandardServer.java
src/com/goldencode/p2j/main/TemporaryAccount.java
src/com/goldencode/p2j/net/Conversation.java
src/com/goldencode/p2j/util/BlockManager.java
src/com/goldencode/p2j/util/ConfigItem.java
src/com/goldencode/p2j/util/TransactionManager.java
src/com/goldencode/p2j/util/UnsupportedMetadataException.java
src/com/goldencode/p2j/security/Authenticator.java

src/com/goldencode/p2j/directory/RedisConfig.java
  • no actual changes, please revert to trunk rev
    src/com/goldencode/p2j/directory/AttributeDefinition.java
src/com/goldencode/p2j/directory/LockManagerTest.java
src/com/goldencode/p2j/directory/ObjectClass.java
src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebSocket.java
src/com/goldencode/p2j/util/Utils.java
  • run javadoc and fix problems in your files (and what Ovidiu mentioned)
  • put the project in your IDE with a i.e. subversion repository or something else, where the base revision is 4369a's trunk revision and you copy 4369a over the project. This allows you to always work with the diff compared with the trunk revision, and not rely on meld or something else to review changes, it is a lot easier to cleanup and ensure formatting, and also you focus on the changes in the branch as a whole, and not individual files.
  • please work on the functional issues above, and after that cleanup formatting/javadoc/etc (review all files as mentioned above).
  • I have not gone through /distributed and other new files related to redis - I'll post a separate note.

#180 Updated by Eduard Soltan about 2 months ago

Committed on 4369a, rev. 16616.

I think this handles everything.

About the spring-dependecy:

=== modified file 'build.gradle'
--- old/build.gradle    2026-05-13 14:02:07 +0000
+++ new/build.gradle    2026-05-25 05:55:10 +0000
@@ -1450,6 +1451,8 @@
     exclude group: 'commons-logging', module: "commons-logging" 
     exclude group: 'org.slf4j', module: 'slf4j-simple'
     exclude group: 'xerces'
+    
+    exclude group: 'org.springframework', module: 'spring-jcl'

     // remove apis already provided by java 11+
     exclude group: 'xml-apis', module: "xml-apis"

I will retest today the app for which clustering is needed, and see how the needed dependencies could be brought.

#181 Updated by Constantin Asofiei about 2 months ago

Eduard Soltan wrote:

Committed on 4369a, rev. 16616.

I think this handles everything.

Did you address the javadocs for new files and other inconsistencies in history entries, etc for existing files? There are still files in /distributed packages which are missing javadocs for methods and such.

#182 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

Eduard Soltan wrote:

Committed on 4369a, rev. 16616.

I think this handles everything.

Did you address the javadocs for new files and other inconsistencies in history entries, etc for existing files? There are still files in /distributed packages which are missing javadocs for methods and such.

Yes, in rev. 16620.

#183 Updated by Constantin Asofiei about 2 months ago

Eduard, good work, there are only a few more issues; I'll run ETF and ChUI with this. Please also rebase. Bellow is review of rev 16620:
  • Module name is incorrect in:
    • UnsupportedMetadataException.java
    • TempGroupAccount.java
    • CreateAccountTask.java
    • LockStatusImpl.java
  • TransactionManager.java - the date in the history entry is wrong: 2020516
  • GuiWebSocket.java needs to be reverted to trunk revision
  • SecurityManagerAuthenticator - missing javadoc for tempUser param in serverAuthHook
  • SecurityManager
    • unused com.goldencode.p2j.main.CreateAccountTask.TempIdManager; import
  • SecurityContext
    • missing tempUser javadoc for SecurityContext constructor
  • SecurityCache
    • no synchronization for creating the tempGroup field (but I think is synchronized via CreateAccountTask.createGroupIfNotExists?)
    • addTempGroup needs to not overwrite the tempGroup field (only one temp group needs ever to exist). I think we can abend if such case happens (now this is protected via the call stack)
    • readUsers - this removes oidcUuid and email fields?
    • readUsers - this line was changed from if (acc != null && acc instanceof GroupAccount) to if (acc != null) ?
  • Authenticator
    • serverAuthHook(byte[] auth, String entity, boolean tempUser) needs to call serverAuthHook(byte[] auth, String entity); by default
  • Account.java
    • the history entry no longer matches the changes.
  • CreateAccountTask.java
    • when you rename a file, you need to use bzr move and not copy-paste the file - history will be lost otherwise. We can't fix it at this point.
  • LocalLockManger
    • Copyright year incorrect
  • GeodeUpdateListener
    • history entry year is wrong

#184 Updated by Constantin Asofiei about 2 months ago

Also: please rebase when convenient so other projects can be tested.

#185 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

  • GuiWebSocket.java needs to be reverted to trunk revision

Meant to solve some javadoc issues, but I see that in latest trunk it has been solved.

  • SecurityCache
    • no synchronization for creating the tempGroup field (but I think is synchronized via CreateAccountTask.createGroupIfNotExists?)

Yes, changes to temp users are made through CreateAccountTask.

  • readUsers - this removes oidcUuid and email fields?

Was meant for readTempUsers.

Rebased 4369a, to trunk rev. 16582. Fixed review issues, and a small code fix that appeared in a customer app. Going to run chui this night.

#186 Updated by Constantin Asofiei about 2 months ago

4369a is OK. ChUI passed, I'll let you know about ETF.

Eduard: we need to figure out the spring library version we need to use, and maybe other jars which collide with the clustering app. Make a list and lets discuss.

#187 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

4369a is OK. ChUI passed, I'll let you know about ETF.

I have tested ETF and chui on my side, they both pass.

#188 Updated by Constantin Asofiei about 2 months ago

Eduard Soltan wrote:

I have tested ETF and chui on my side, they both pass.

Great, then I won't bother with ETF.

Additional testing: in multi-tenant mode and with i.e. 150 or 200 classic agents started, without clustering - i.e. test if agents start properly, if you haven't already.

#189 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

Additional testing: in multi-tenant mode and with i.e. 150 or 200 classic agents started, without clustering - i.e. test if agents start properly, if you haven't already.

Yes, and rev. 16634 contains a small fix which prevents a deadlock in DistributedInMemoryLockManager.release

#190 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

We need to move to 6.2.18 version in build.gradle - is this possible?

I looked over difference of jar dependencies in multi tenancy application.

These are dependencies added by 4369a, compared to trunk:

4369a Multi tenancy app
commons-pool2-2.12.0.jar commons-pool2-2.12.1.jar
gson-2.10.1.jar gson-2.13.1.jar
HdrHistogram-2.1.12.jar HdrHistogram-2.2.2.jar
jackson-datatype-jsr310-2.18.3.jar jackson-datatype-jsr310-2.21.2.jar
jedis-5.1.0.jar jedis-5.1.5.jar
json-20231013.jar json-20250107.jar
micrometer-core-1.9.0.jar micrometer-core-1.15.11.jar
reactor-core-3.6.2.jar reactor-core-3.7.6.jar
spring-beans-5.3.20.jar spring-beans-6.2.18.jar
spring-core-5.3.20.jar spring-core-6.2.18.jar
spring-web-5.3.20.jar spring-web-6.2.18.jar

#191 Updated by Eduard Soltan about 2 months ago 

=== modified file 'build.gradle'
--- old/build.gradle    2026-05-26 19:40:32 +0000
+++ new/build.gradle    2026-05-27 15:06:53 +0000
@@ -1468,6 +1468,21 @@
        attribute(TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE,
                     objects.named(TargetJvmEnvironment, TargetJvmEnvironment.STANDARD_JVM))
     }
+    
+    resolutionStrategy.eachDependency { details ->
+        def group = details.requested.group
+        def name = details.requested.name
+        
+        if (group == 'org.springframework' && ['spring-beans', 'spring-core', 'spring-web'].contains(name)) {
+            details.useVersion('6.2.18')
+        } else if (group == 'org.apache.commons' && name == 'commons-pool2') {
+            details.useVersion('2.12.1')
+        } else if (group == 'com.google.code.gson' && name == 'gson') {
+            details.useVersion('2.13.1')
+        } else if (group == 'org.json' && name == 'json') {
+            details.useVersion('20250107')
+        }
+    }
 }

 // Wire java plugin classpath configurations to existing FWD dependency configs
@@ -1648,7 +1663,7 @@
     fwdServer group: 'com.kohlschutter.junixsocket', name: 'junixsocket-common', version: '2.8.3'
     fwdServer group: 'com.kohlschutter.junixsocket', name: 'junixsocket-native-common', version: '2.8.3'
     fwdServer group: 'org.apache.geode', name: 'geode-core', version: '1.15.1'
-    fwdServer group: 'redis.clients', name: 'jedis', version: '5.1.0'
+    fwdServer group: 'redis.clients', name: 'jedis', version: '5.1.5'
     fwdServer group: 'org.redisson', name: 'redisson', version: '3.27.2'

     fwdClient group: 'org.apache.xmlgraphics', name: 'batik-svggen', version: '1.18'

This should solve some versioning problem.

#192 Updated by Constantin Asofiei about 2 months ago

Eduard, is this gradle change in the branch? What about the other jars?

Can we finish testing today for the other 2 apps?

#193 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

Eduard, is this gradle change in the branch? What about the other jars?

I committed on rev. 16635, plus a changes s.t VSTs to be enabled by default.

Can we finish testing today for the other 2 apps?

Large GUI application unit tests I tested them myself and it was OK, awaiting results for smoke tests.

For other GUI application, I was informed that smoke tests were OK.

#194 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

Eduard, is this gradle change in the branch? What about the other jars?

I upgraded the version for all jars, except for jackson-datatype-jsr310-2.21.2.jar. All other jackson related libraries in 4369a and in customer classpath uses 2.18.3, so I don't think this is a problem.

#195 Updated by Constantin Asofiei about 2 months ago

Did you do a security scanning of the other jars brought by geode in 4369a?

#196 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

Did you do a security scanning of the other jars brought by geode in 4369a?

Yes, there was some critical or high vulnerability for geode-core-1.15.1.jar and its shiro dependencies. Switch to geode-core-1.15.2.jar and they were fixed.

#197 Updated by Constantin Asofiei about 2 months ago

Eduard Soltan wrote:

Constantin Asofiei wrote:

Did you do a security scanning of the other jars brought by geode in 4369a?

Yes, there was some critical or high vulnerability for geode-core-1.15.1.jar and its shiro dependencies. Switch to geode-core-1.15.2.jar and they were fixed.

OK, anything else left to test or work?

#198 Updated by Eduard Soltan about 2 months ago

Constantin Asofiei wrote:

OK, anything else left to test or work?

Waiting for large gui application smoke tests, and I will retest chui just in case.

Please look at https://proj.goldencode.com/projects/p2j/wiki/Stateless_Server_Clustering#section-10

#199 Updated by Greg Shah about 2 months ago

Did you change the default setting for metadata (at least in non-clustered mode)?

We need clear documentation on how to "opt-out" of metadata support. And we are waiting on Ovidiu's thoughts on the metadata configuration.

#200 Updated by Eduard Soltan about 2 months ago

Greg Shah wrote:

Did you change the default setting for metadata (at least in non-clustered mode)?

Yes, now it is enabled by default.

#201 Updated by Eduard Soltan about 2 months ago

Eduard Soltan wrote:

Waiting for large gui application smoke tests, and I will retest chui just in case.

Large Gui application passed.

#202 Updated by Constantin Asofiei about 2 months ago

Is the wiki complete? Ovidiu, can you take a look, too? Stateless_Server_Clustering

#203 Updated by Ovidiu Maxiniuc about 2 months ago

Greg Shah wrote:

[...] we are waiting on Ovidiu's thoughts on the metadata configuration.

Yes, I am a bit concerned about having this configured in multiple places:
  • starting with the conversion (if not specified in respective namespace of p2j.cfg.xml, the tables will not be converted therefore not available at runtime);
  • then there is the useMeta in DatabaseManager. Even if the meta tables are available, at any error when the _meta manager is initialised, it will deactivate these tables globally. That includes the case when the p2j.cfg.xml is modified post-conversion;
  • this comes at least as a third layer, individually configurable in directory for lock-table, connection-table, ... and read by MetaConfig;
  • LE: this 3rd bullet is configurable per node and might cause issues if not all nodes are configured the same.

As long as the metadata support is on by default, I think we can keep it in current form. We can handle this later as this is not a showstopper for this task. I slight double that a customer will try do deactivate them. From my knowledge, all the projects large enough to want to distribute FWD do require access to meta database. So, having these on by default will simplify the directory and administrator's job.

Note: I hereby refer to src/com/goldencode/p2j/persist/meta/config/MetaConfig.java. The other newly introduced configuration options are not a problem.

#204 Updated by Eduard Soltan about 2 months ago

Ovidiu Maxiniuc wrote:

  • then there is the useMeta in DatabaseManager. Even if the meta tables are available, at any error when the _meta manager is initialised, it will deactivate these tables globally. That includes the case when the p2j.cfg.xml is modified post-conversion;

And we have to rebuild the app in order to disable it?

#205 Updated by Ovidiu Maxiniuc about 2 months ago

Constantin Asofiei wrote:

Is the wiki complete? Ovidiu, can you take a look, too? Stateless_Server_Clustering

Review and ideas:
  • (typo) the capitalisation of headers;
  • in Solution for synchronization section, we could replace the bold text with links to respective projects;
  • (inconsistency) there is no chapter 2. The following chapter names are not numbered at all;
  • in 3.1 the following phrase is repeated twice in subsequent phrases: switch to client/server mode if needed. Although not incorrect, it sounds odd;
  • (typo) there is a connection connection in the phrase above mentioned;
  • in Redis Directory paragraph, I would use code for RedisRemapper and RedisLockManager identifiers;
  • maybe the architecture should be a bit more emphasised. The localhost and 10334 and 10335 ports are used across the document, but the not visually. I think it's better to start with an example and use it along the document. From initial introduction up to later configurations;
  • using only 2 nodes might be OK for client-server, but not be enough to understand the peer-to-peer architecture/communication;
  • Meta Tables disabling. Currently, it seems like an advised think to do. However, it is not specified: what are the benefits and downsides. Let the customer the choice!

I might add supplementary comments when I will do the setup myself and encounter actual issues (they always occur!) :).

#206 Updated by Constantin Asofiei about 2 months ago

Ovidiu Maxiniuc wrote:

Greg Shah wrote:

[...] we are waiting on Ovidiu's thoughts on the metadata configuration.

Yes, I am a bit concerned about having this configured in multiple places:
  • starting with the conversion (if not specified in respective namespace of p2j.cfg.xml, the tables will not be converted therefore not available at runtime);

If we don't enable these meta tables, then if the conversion uses them statically, then the conversion will fail.

  • then there is the useMeta in DatabaseManager. Even if the meta tables are available, at any error when the _meta manager is initialised, it will deactivate these tables globally. That includes the case when the p2j.cfg.xml is modified post-conversion;

I assume runtime will fail in this case.

  • this comes at least as a third layer, individually configurable in directory for lock-table, connection-table, ... and read by MetaConfig;

Yes. This is usable in a scenario where you want to see where/how the application fails by disabling certain meta tables, as a STOP condition will be raised if is used. This will 'catch' both static and dynamic queries for meta tables, while conversion will 'catch' only the static usage.

#207 Updated by Constantin Asofiei about 2 months ago

  • Status changed from Review to Merge Pending

Eduard: please merge 4369a now.

#208 Updated by Eduard Soltan about 2 months ago

  • Status changed from Merge Pending to Test

4369a was merged to trunk rev. 16588 and archived.

Also available in: Atom PDF