[
https://issues.apache.org/jira/browse/FINERACT-1493?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Aleksandar Vidakovic updated FINERACT-1493:
-------------------------------------------
Description:
Make multi-tenancy optional. The way it is implemented and the fact that the
"TenancyUtil" is used in pretty much all cache annotations slows down
processing in general quite considerably. This could be achieved by introducing
properties file based configuration of tenants. Each tenant would have his own
application-xxx.properties file ("xxx" indicating the tenant ID). This would
allow dev-ops and admins to de-/activate tenants based on file configurations
and doesn't require to setup a separate database for the tenant configuration
beforehand.
In case we have only 1 tenant (which is probably happening in 90% of the
production deployments) then we can even replace the routing datasource (see
[https://github.com/apache/fineract/blame/develop/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/RoutingDataSource.java]).
The one tenant scenario would allow us to configure a "normal" data source
that doesn't need to lookup the tenant information on each request that hits
the system. Other potential issues related to this are memory leaks (if the
tenant information is not properly removed) and preventing us to take full
advantage of virtual threads (thread local variables are still inherited from
their native thread parents... scoped values in JDK 25 are a better solution,
needs a separate Jira ticket).
The current tenant system has also implications on the cache configuration.
Example from the current codebase:
{code:java}
@Cacheable(key =
"T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat('calId_'
+ #calendarId + '{_}entityId{_}' + #entityId + '{_}entityTypeId{_}' +
#entityTypeId)")
{code}
Pretty much every single cache annotation we have contains SpEL expressions
like these. In turn that means that every single cache lookup (or storage) is
using thread local variables to lookup the current tenant details (here only
tenant ID is needed to ensure that each tenant has an isolated cache and values
don't leak between tenants). The first thing we should do here - no matter if
we change the tenant mechanics or not - is to configure the separate tenant
caches in a central Spring Java configuration instead of dragging this concern
into each annotation in the code base; this makes life (read: maintenance)
already a lot easier. If we had to change then something then we could do this
in one central place and don't have to touch pretty much the whole code base.
Once that is done then we can introduce conditional cache configurations for
multi tenant (use of thread local variables as before) and single tenant (no
use of thread local variables) systems.
Such a configuration could look like:
{code:java}
public class TenantAwareCache implements Cache {
private final CacheManager delegate;
private final String name;
public TenantAwareCache(CacheManager delegate, String name) {
this.delegate = delegate;
this.name = name;
}
/** Resolve the underlying cache for the current tenant. */
private Cache current() {
String tenant =
ThreadLocalContextUtil.getTenant().getTenantIdentifier();
// Each tenant gets its own isolated cache region.
return delegate.getCache(name + "::" + tenant);
}
@Override public String getName() { return name; }
@Override public Object getNativeCache() { return
current().getNativeCache(); }
@Override public ValueWrapper get(Object key) { return
current().get(key); }
@Override public <T> T get(Object key, Class<T> type) { return
current().get(key, type); }
@Override public <T> T get(Object key, Callable<T> loader) { return
current().get(key, loader); }
@Override public void put(Object key, Object value) {
current().put(key, value); }
@Override public ValueWrapper putIfAbsent(Object key, Object v){ return
current().putIfAbsent(key, v); }
@Override public void evict(Object key) {
current().evict(key); }
@Override public boolean evictIfPresent(Object key) { return
current().evictIfPresent(key); }
@Override public void clear() {
current().clear(); } // per-tenant!
@Override public boolean invalidate() { return
current().invalidate(); }
}
{code}
{code:java}
public class TenantAwareCacheManager extends AbstractCacheManager {
private final CacheManager delegate;
public TenantAwareCacheManager(CacheManager delegate) {
this.delegate = delegate;
}
@Override
protected Collection<? extends Cache> loadCaches() {
List<Cache> caches = new ArrayList<>();
for (String name : delegate.getCacheNames()) {
caches.add(new TenantAwareCache(delegate, name));
}
return caches;
}
@Override
protected Cache getMissingCache(String name) {
return new TenantAwareCache(delegate, name);
}
}
{code}
{code:java}
@Configuration
@EnableCaching
public class CacheConfiguration {
@Bean
public CacheManager cacheManager() {
// Any delegate works: Caffeine, ConcurrentMapCacheManager,
RedisCacheManager, ...
CaffeineCacheManager delegate = new CaffeineCacheManager();
delegate.setCaffeine(
Caffeine.newBuilder()
.expireAfterWrite(10, TimeUnit.MINUTES)
.maximumSize(10_000)
);
// Allow dynamic per-tenant cache creation (default is true).
delegate.setAsyncCacheMode(false);
return new TenantAwareCacheManager(delegate);
}
}
{code}
This would allow us to simplify the cache annoations to (using previous -
already complex - example):
{code:java}
@Cacheable(key = "'calId_' + #calendarId + '{_}entityId{_}' + #entityId +
'{_}entityTypeId{_}' + #entityTypeId")
{code}
The immediate benefit is that we reduce the visibility and dependency on the
"ThreadLocalContextUtil" by a lot; currently this dependency is pretty much
used/visible all over the source code via the cache annotations. Having this
localized dependency in the tenant aware cache manager simplifies then also the
future migration to scoped values to replace the use of thread local variables.
Again, depending on the one tenant or multi tenant scenario we could then
either configure the tenant aware cache or go with the "normal" cache
configuration (no thread local variable lookups).
was:
Make multi-tenancy optional. The way it is implemented and the fact that the
"TenancyUtil" is used in pretty much all cache annotations slows down
processing in general quite considerably. This could be achieved by introducing
properties file based configuration of tenants. Each tenant would have his own
application-xxx.properties file ("xxx" indicating the tenant ID). This would
allow dev-ops and admins to de-/activate tenants based on file configurations
and doesn't require to setup a separate database for the tenant configuration
beforehand.
In case we have only 1 tenant (which is probably happening in 90% of the
production deployments) then we can even replace the routing datasource (see
[https://github.com/apache/fineract/blame/develop/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/RoutingDataSource.java]).
The one tenant scenario would allow us to configure a "normal" data source
that doesn't need to lookup the tenant information on each request that hits
the system. Other potential issues related to this are memory leaks (if the
tenant information is not properly removed) and preventing us to take full
advantage of virtual threads (thread local variables are still inherited from
their native thread parents... scoped values in JDK 25 are a better solution,
needs a separate Jira ticket).
The current tenant system has also implications on the cache configuration.
Example from the current codebase:
{code:java}
@Cacheable(key =
"T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat('calId_'
+ #calendarId + '{_}entityId{_}' + #entityId + '{_}entityTypeId{_}' +
#entityTypeId)")
{code}
Pretty much every single cache annotation we have contains SpEL expressions
like these. In turn that means that every single cache lookup (or storage) is
using thread local variables to lookup the current tenant details (here only
tenant ID is needed to ensure that each tenant has an isolated cache and values
don't leak between tenants). The first thing we should do here - no matter if
we change the tenant mechanics or not - is to configure the separate tenant
caches in a central Spring Java configuration instead of dragging this concern
into each annotation in the code base; this makes life (read: maintenance)
already a lot easier. If we had to change then something then we could do this
in one central place and don't have to touch pretty much the whole code base.
Once that is done then we can introduce conditional cache configurations for
multi tenant (use of thread local variables as before) and single tenant (no
use of thread local variables) systems.
Such a configuration could look like:
{code:java}
public class TenantAwareCache implements Cache {
private final CacheManager delegate;
private final String name;
public TenantAwareCache(CacheManager delegate, String name) {
this.delegate = delegate;
this.name = name;
}
/** Resolve the underlying cache for the current tenant. */
private Cache current() {
String tenant =
ThreadLocalContextUtil.getTenant().getTenantIdentifier();
// Each tenant gets its own isolated cache region.
return delegate.getCache(name + "::" + tenant);
}
@Override public String getName() { return name; }
@Override public Object getNativeCache() { return
current().getNativeCache(); }
@Override public ValueWrapper get(Object key) { return
current().get(key); }
@Override public <T> T get(Object key, Class<T> type) { return
current().get(key, type); }
@Override public <T> T get(Object key, Callable<T> loader) { return
current().get(key, loader); }
@Override public void put(Object key, Object value) {
current().put(key, value); }
@Override public ValueWrapper putIfAbsent(Object key, Object v){ return
current().putIfAbsent(key, v); }
@Override public void evict(Object key) {
current().evict(key); }
@Override public boolean evictIfPresent(Object key) { return
current().evictIfPresent(key); }
@Override public void clear() {
current().clear(); } // per-tenant!
@Override public boolean invalidate() { return
current().invalidate(); }
}
{code}
{code:java}
public class TenantAwareCacheManager extends AbstractCacheManager {
private final CacheManager delegate;
public TenantAwareCacheManager(CacheManager delegate) {
this.delegate = delegate;
}
@Override
protected Collection<? extends Cache> loadCaches() {
List<Cache> caches = new ArrayList<>();
for (String name : delegate.getCacheNames()) {
caches.add(new TenantAwareCache(delegate, name));
}
return caches;
}
@Override
protected Cache getMissingCache(String name) {
return new TenantAwareCache(delegate, name);
}
}
{code}
{code:java}
@Configuration
@EnableCaching
public class CacheConfiguration {
@Bean
public CacheManager cacheManager() {
// Any delegate works: Caffeine, ConcurrentMapCacheManager,
RedisCacheManager, ...
CaffeineCacheManager delegate = new CaffeineCacheManager();
delegate.setCaffeine(
Caffeine.newBuilder()
.expireAfterWrite(10, TimeUnit.MINUTES)
.maximumSize(10_000)
);
// Allow dynamic per-tenant cache creation (default is true).
delegate.setAsyncCacheMode(false);
return new TenantAwareCacheManager(delegate);
}
}
{code}
This would allow us to simplify the cache annoations to (using previous -
already complex - example):
{code:java}
@Cacheable(key = "'calId_' + #calendarId + '{_}entityId{_}' + #entityId +
'{_}entityTypeId{_}' + #entityTypeId")
{code}
> Configurable single and multi tenancy
> -------------------------------------
>
> Key: FINERACT-1493
> URL: https://issues.apache.org/jira/browse/FINERACT-1493
> Project: Apache Fineract
> Issue Type: New Feature
> Reporter: Aleksandar Vidakovic
> Assignee: Aleksandar Vidakovic
> Priority: Major
> Fix For: 1.16.0
>
>
> Make multi-tenancy optional. The way it is implemented and the fact that the
> "TenancyUtil" is used in pretty much all cache annotations slows down
> processing in general quite considerably. This could be achieved by
> introducing properties file based configuration of tenants. Each tenant would
> have his own application-xxx.properties file ("xxx" indicating the tenant
> ID). This would allow dev-ops and admins to de-/activate tenants based on
> file configurations and doesn't require to setup a separate database for the
> tenant configuration beforehand.
> In case we have only 1 tenant (which is probably happening in 90% of the
> production deployments) then we can even replace the routing datasource (see
> [https://github.com/apache/fineract/blame/develop/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/RoutingDataSource.java]).
> The one tenant scenario would allow us to configure a "normal" data source
> that doesn't need to lookup the tenant information on each request that hits
> the system. Other potential issues related to this are memory leaks (if the
> tenant information is not properly removed) and preventing us to take full
> advantage of virtual threads (thread local variables are still inherited from
> their native thread parents... scoped values in JDK 25 are a better solution,
> needs a separate Jira ticket).
> The current tenant system has also implications on the cache configuration.
> Example from the current codebase:
> {code:java}
> @Cacheable(key =
> "T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat('calId_'
> + #calendarId + '{_}entityId{_}' + #entityId + '{_}entityTypeId{_}' +
> #entityTypeId)")
> {code}
> Pretty much every single cache annotation we have contains SpEL expressions
> like these. In turn that means that every single cache lookup (or storage) is
> using thread local variables to lookup the current tenant details (here only
> tenant ID is needed to ensure that each tenant has an isolated cache and
> values don't leak between tenants). The first thing we should do here - no
> matter if we change the tenant mechanics or not - is to configure the
> separate tenant caches in a central Spring Java configuration instead of
> dragging this concern into each annotation in the code base; this makes life
> (read: maintenance) already a lot easier. If we had to change then something
> then we could do this in one central place and don't have to touch pretty
> much the whole code base. Once that is done then we can introduce conditional
> cache configurations for multi tenant (use of thread local variables as
> before) and single tenant (no use of thread local variables) systems.
> Such a configuration could look like:
> {code:java}
> public class TenantAwareCache implements Cache {
> private final CacheManager delegate;
> private final String name;
> public TenantAwareCache(CacheManager delegate, String name) {
> this.delegate = delegate;
> this.name = name;
> }
> /** Resolve the underlying cache for the current tenant. */
> private Cache current() {
> String tenant =
> ThreadLocalContextUtil.getTenant().getTenantIdentifier();
> // Each tenant gets its own isolated cache region.
> return delegate.getCache(name + "::" + tenant);
> }
> @Override public String getName() { return name; }
> @Override public Object getNativeCache() { return
> current().getNativeCache(); }
> @Override public ValueWrapper get(Object key) { return
> current().get(key); }
> @Override public <T> T get(Object key, Class<T> type) { return
> current().get(key, type); }
> @Override public <T> T get(Object key, Callable<T> loader) { return
> current().get(key, loader); }
> @Override public void put(Object key, Object value) {
> current().put(key, value); }
> @Override public ValueWrapper putIfAbsent(Object key, Object v){ return
> current().putIfAbsent(key, v); }
> @Override public void evict(Object key) {
> current().evict(key); }
> @Override public boolean evictIfPresent(Object key) { return
> current().evictIfPresent(key); }
> @Override public void clear() {
> current().clear(); } // per-tenant!
> @Override public boolean invalidate() { return
> current().invalidate(); }
> }
> {code}
> {code:java}
> public class TenantAwareCacheManager extends AbstractCacheManager {
> private final CacheManager delegate;
> public TenantAwareCacheManager(CacheManager delegate) {
> this.delegate = delegate;
> }
> @Override
> protected Collection<? extends Cache> loadCaches() {
> List<Cache> caches = new ArrayList<>();
> for (String name : delegate.getCacheNames()) {
> caches.add(new TenantAwareCache(delegate, name));
> }
> return caches;
> }
> @Override
> protected Cache getMissingCache(String name) {
> return new TenantAwareCache(delegate, name);
> }
> }
> {code}
> {code:java}
> @Configuration
> @EnableCaching
> public class CacheConfiguration {
> @Bean
> public CacheManager cacheManager() {
> // Any delegate works: Caffeine, ConcurrentMapCacheManager,
> RedisCacheManager, ...
> CaffeineCacheManager delegate = new CaffeineCacheManager();
> delegate.setCaffeine(
> Caffeine.newBuilder()
> .expireAfterWrite(10, TimeUnit.MINUTES)
> .maximumSize(10_000)
> );
> // Allow dynamic per-tenant cache creation (default is true).
> delegate.setAsyncCacheMode(false);
> return new TenantAwareCacheManager(delegate);
> }
> }
> {code}
> This would allow us to simplify the cache annoations to (using previous -
> already complex - example):
> {code:java}
> @Cacheable(key = "'calId_' + #calendarId + '{_}entityId{_}' + #entityId +
> '{_}entityTypeId{_}' + #entityTypeId")
> {code}
> The immediate benefit is that we reduce the visibility and dependency on the
> "ThreadLocalContextUtil" by a lot; currently this dependency is pretty much
> used/visible all over the source code via the cache annotations. Having this
> localized dependency in the tenant aware cache manager simplifies then also
> the future migration to scoped values to replace the use of thread local
> variables.
> Again, depending on the one tenant or multi tenant scenario we could then
> either configure the tenant aware cache or go with the "normal" cache
> configuration (no thread local variable lookups).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
