Provide more granular configuration options to choose DAO implementations

[aravindanr]

Currently the DAO (MetadataDAO, QueueDAO, ExecutionDAO) implementations are chosen by one property called db. This prevents users from picking different implementations for different purposes. For instance, executions could be stored in Cassandra, but queue could be MySQL.

One approach is to provide a configuration property for each DAO interface.

[marcocrasso]

@aravindanr
I'd like to contribute with this, however I had a couple of questions after diving at 3.0 source code and #2016. Based on available DAOs and implementations found in this repo, I sketched this:

Based on "One approach is to provide a configuration property for each DAO interface." and the fact that there are seven DAOs, but we can ignore IndexDAO here, using a granularity level of 6 would mean going from:

conductor.db.type=[memory | dynomite | redis* | postgres | mysql]

to:

conductor.db.metadata.type=[ cassandra | memory | dynomite | redis* | postgres | mysql ]
conductor.db.eventhandler.type= [ cassandra | memory |dynomite | redis* | postgres | mysql ]
conductor.db.polldata.type=[ cassandra | memory | dynomite | redis* | postgres | mysql ]
conductor.db.execution.type=[ cassandra | memory| dynomite | redis* | postgres | mysql ]
conductor.db.queue.type=[  memory | dynomite | redis* | postgres | mysql ]
conductor.db.ratelimiting.type=[ memory | dynomite | redis* | postgres | mysql ]

where "redis*" eq redis_standalone | redis_cluster | redis_sentinel

Are the DAO implementations fully independent from each other? Is achievable this approach?

Or, alternatively the granularity level should be 3: MetadataDAO, QueueDAO, ExecutionDAO

conductor.db.metadata.type=[ cassandra | memory | dynomite | redis* | postgres | mysql ]
conductor.db.polldata.type=[ cassandra | memory | dynomite | redis* | postgres | mysql ]
conductor.db.execution.type=[ cassandra | memory| dynomite | redis* | postgres | mysql ]

in which case eventhandler, polldata, and ratelimiting should inherit the configuration due to the intrinsic dependencies.

Any suggestion here will allow me to move forward. I am can extend current support using ConditionalOnExpression on each Bean declaration. Regards

[aravindanr]

@marcocrasso Thanks for volunteering. The idea behind this is to not eliminate conductor.db.type, but allow overrides for it. For instance, in #2016, the ask was to use MySQL for definitions & executions but Redis for queues. In the current setup, that is not possible.

The implementation that i am aiming for is something like,

conductor.db.type=mysql
com.netflix.conductor.dao.QueueDAO=<fully qualified class name of a class that implements QueueDAO>

The effect of the above configuration should be, for all DAO implementations except QueueDAO, the MySQL implementations are chosen. It is important that another layer of grouping is not introduced. With this override, users have greater flexibility in choosing DAO implementations, while somewhat retaining the conciseness that a grouping like db.type offers.

The reason for using the fully qualified class names in the key and value is, in the future, the DAO contracts can be published as an SPI module and implementations are loaded during runtime using ServiceLoader.

Hope that clarifies the intention.

[marcocrasso]

@aravindanr
I've refactored all x-persistence modules to implement these as Java SPI modules, added one new module named service-loader which loads SPI Service Providers as spring-boot beans, updated documentation and added docker-compose-* with backend combinations, like MySQL +Cassandra.

I have forked the repo. Right now, me and our team is testing the Cassandra + MySQL combination. I decided to fork it, since the refactoring was big. Once tested and confirmed everything is working as expected, my next step would be to submit a PR. I assigned the issue to myself, hope that is ok.

Changes

Main work was about replacing the DI mechanism used for backend, spring-boot Conditional beans with Java SPI. This requires to load beans into spring-boot context differently (not more relying on ConditionalBeans, and to assume that two or more implementations can co-exist in the class loader without causing beans collisions), and to accommodate those modules providing DAOs implementations (i.e. the x-persistence ones) to Java SPI impositions, such default no args constructor. I addressed the backward compatibility goal too.

This class holds the new beans mechanism service-loader

By backward compatibility I meant supporting beans configuration using the previous “one-property” approach.

Hard Constraints

I auto-imposed as a hard constraint NOT TO TOUCH core module. My rationale was that we are replacing how beans are loaded into spring-boot context, and nothing else. Therefore, core must keep working without noticing these if and only if required beans can be auto-wired.

Documentation:

Overview of backend as Java SPI

https://github.com/marcocrasso/conductor/tree/iss2017_granular_spi_module_configuration/service-loader#service-provider-loader

Migration guide

https://github.com/marcocrasso/conductor/tree/iss2017_granular_spi_module_configuration/service-loader#persistence-module-migration-guide how to move an existence *-persistence module to the new Java SPI-based approach.

Updated Oficial docs

https://github.com/marcocrasso/conductor/blob/iss2017_granular_spi_module_configuration/docs/docs/extend.md I extended official project documentation in section “extend” to explain how to add a new backend implementation.

Running on Docker

https://github.com/marcocrasso/conductor/tree/iss2017_granular_spi_module_configuration/docker#readme completely updated Docker steps to show how to use the system with Cassandra, MySQL, Redis, or combinations; all on top of docker-compose.

Testing

All tests, both individual and integrations, are passing. Further testing is ongoing.

[aravindanr]

@marcocrasso Thanks for taking the time to do this and also factoring in backwards compatibility. In the service-loader package, we could invent our own service loader that does not rely on Java's ServiceLoader and its requirements.

ServiceLoader brings in two hard requirements that classes need to be mentioned in the META-INF folder and also contain a no-arg constructor. We also expect Conductor users to specify fully qualified class names as part of conductor config file. That's repeated configuration and more prone to errors. Users can miss specifying in one of the places. Also the fact that we need to resort to field injection because ServiceLoader requires a no-arg constructor.

We propose that the beans are registered dynamically using a GenericBeanDefinition in a BeanFactoryPostProcessor. Something like below,

@Component
@DependsOn("BackwardCompatibleConfiguration")
public class SPIBeanFactoryPostProcessor implements BeanFactoryPostProcessor {

    private static final Logger log = LoggerFactory.getLogger(SPIBeanFactoryPostProcessor.class);

    @Value("${com.netflix.conductor.dao.MetadataDAO:}")
    private String metadataDAOType;

    // other properties go here

    @Override
    public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
        if(StringUtils.isNotEmpty(metadataDAOType)) {
            registerService(MetadataDAO.class, metadataDAOType, (DefaultListableBeanFactory) beanFactory);
        }

        // the above snippet is repeated for all types
    }

    protected void registerService(Class<?> typeKey, String expectedType, DefaultListableBeanFactory beanFactory) {
        Assert.notNull(expectedType, MessageFormat.format("Expected SPI type cannot be null. Check your configuration key {0}", typeKey.getName()));
        Assert.hasText(expectedType, MessageFormat.format("Expected SPI type cannot be empty. Check your configuration key {0}", typeKey.getName()));
        try {
            Class<?> expectedClass = Class.forName(expectedType);

            GenericBeanDefinition gbd = new GenericBeanDefinition();
            gbd.setBeanClass(expectedClass);
            String beanName = Introspector.decapitalize(expectedClass.getSimpleName());
            beanFactory.registerBeanDefinition(beanName, gbd);
        } catch (ClassNotFoundException e) {
            log.info("Couldn't find class '{}' in the classpath while loading '{}'. Check build.gradle 'dependencies' section", expectedType, typeKey.getName());
            throw new RuntimeException();
        }
    }
}

This eliminates both the shortcomings of ServiceLoader. Let us know what you think.

[marcocrasso]

Hi @aravindanr ! sorry for the delay in responding. First, you are welcome! I am really enjoying this, right now I am in the middle of a load test, hopefully we will able to share empirical results about which storage layer (or combinations) works better under different load scenarios.

I have been thinking on this since I read your post. There a couple of things that believe important to be clarified before exposing my thoughts.

1. Java SPI also supports constructors with arguments, quoting : "If the service provider declares a provider method, then the service loader invokes that method to obtain an instance of the service provider. A provider method is a public static method named "provider" with no formal parameters and a return type that is assignable to the service's interface or class. ... If the service provider does not declare a provider method, then the service provider is instantiated directly, via its provider constructor. A provider constructor is a public constructor with no formal parameters. " Oficial docs

2. with Java SPI there are mainly two kind of users: dependants and suppliers. It is expected that suppliers known and control the canonical names of their implementations classes and write those in META folder. On the other side, it is not expected that dependants (e.g. conductor/server users) know the internal of the JAR file containing the supplied service. It is black-box reuse driven by the inclusion of the respective JAR file into the dependencies list.

I think that requiring dependants to write the canonical classes names of desired implementations is not a good idea. It doesn't scale well, goes against black-box reuse, and of course is error prune as you mentioned.

At this point, I can help on any alternative, been my suggestion that: since choosing the storage layer services a responsibility of one lead/senior engineer or devops of Conductor Server module, they must be able to do it just by including the right dependencies in the server project. In such a way:

• black box reuse will be embraced
• no need to know/write canonical classes names at the dependants' side

To accomplish that in a purist way, services JAR must be split, in order to have individual DAO implementation in each SPI module. Otherwise, we cannot combine like: "MetadataDAO with mysql and ExecutionDAO with redis, while the rest with postgres". This will require a lot of work, yes.

Regarding the BeanFactoryPostProcessor, it is ok and removes the limitations you mention but IMHO it has one problem, and I am not saying that before implementation doesn't. This moves from an standard like Java SPI to another custom pattern. It is rare to have combined spring-boot with Java SPI in the first place; but it is a good idea to adhere to Java SPI if you are willing to have more and more DAOs services implementations made by third-parties.

The trade-off I see is:

• would Conductor expect to gain adoption by allowing extending the DAOs layer without knowing just the domain and API of one specific DAO ? Is this extension point of the framework High Priority one?

In case yes, effort is on the other side of the balance. As said, I am ok to do it. The paradigm shift would be that configuring the storage services layer of Conductor will be done by just including JARs in your dependencies list. Example: mysql-metadatadao.jar, cassandra-queuedao.jar, and 4 more...

PS: there are might be other alternatives/ideas for sure. I'll keep thinking.

[maheshyaddanapudi]

@aravindanr great idea and @marcocrasso great work !

If the idea is to segregate the database for each of the implementations, why not also add in another conductor-worflow-scheduler module ? A sample version which I used as separate Conductor WF Scheduler is --> API Calls Scheduler

This was designed and developed for Conductor when it was Guice based and a direct Spring + MySQL + Quartz integration wasn't an easy task. However, as Conductor 3.X is Spring Boot based, it's a very farly easy job to to integrate a scheduler and expose new set of APIs.