apache / shardingsphere-elasticjob

Distributed scheduled job
Apache License 2.0
8.15k stars 3.29k forks source link

Why not support multiple job error handlers? #1622

Open TeslaCN opened 4 years ago

TeslaCN commented 4 years ago

Feature Request

https://github.com/apache/shardingsphere-elasticjob/blob/e1bf8623713742d19500518545848f91b6d9944d/elasticjob-api/src/main/java/org/apache/shardingsphere/elasticjob/api/JobConfiguration.java#L64-L68

Describe the feature you would like.

Remove field "jobErrorHandlerType". The job will have 3 error handlerse if I add 3 instances of ErrorHandlerConfiguration.

terrymanu commented 4 years ago

It is better to support multiple job error handlers. I am not sure we can do it now or after 3.0.0-beta release.

TeslaCN commented 4 years ago

Current API

    private JobConfiguration createJobConfiguration() {
        return JobConfiguration.newBuilder("test_job", 3)
                .cron("0/1 * * * * ?").shardingItemParameters("0=A,1=B,2=C").jobParameter("param").failover(true).misfire(false)
                .jobErrorHandlerType("THROW")
                .description("desc").build();
    }

How about the following API ?

    private JobConfiguration createJobConfiguration() {
        return JobConfiguration.newBuilder("test_job", 3)
                .cron("0/1 * * * * ?").shardingItemParameters("0=A,1=B,2=C").jobParameter("param").failover(true).misfire(false)
                .addExtraConfigurations(new EmailConfiguration())
                .addExtraConfigurations(new DingtalkConfiguration())
                .addExtraConfigurations(new LogErrorHandlerConfiguration())
                .description("desc").build();
    }
terrymanu commented 4 years ago

The configuration of THROW error handler maybe need to discuss. It is in consist with type based config like sharding or executors

TeslaCN commented 4 years ago

How to config THROW error handler in the new API?

THROW is specified by a configuration class without any fields.

public final class ThrowErrorHandlerConfiguration implements ErrorHandlerConfiguration {

    @Override
    public String getType() {
        return "THROW";
    }
}

I can send a email before throw an exception.

                .addExtraConfigurations(new EmailConfiguration())
                .addExtraConfigurations(new ThrowErrorHandlerConfiguration())
                .addExtraConfigurations(new LogErrorHandlerConfiguration()) // Will not be invoked.
TeslaCN commented 4 years ago

I did some changes in https://github.com/TeslaCN/shardingsphere-elasticjob/tree/fixes-1622

terrymanu commented 4 years ago

It is a good idea, maybe you can submit the pull request and we can review your change.

TeslaCN commented 4 years ago

The branch TeslaCN/fix-1622 is base on configuring job error handler by JobExtraConfiguration. Since https://github.com/apache/shardingsphere-elasticjob/pull/1626 has already revert the way to configure job error handler, I think we can further discuss this issue and do this in a future version.