bifromqio / bifromq

A Multi-Tenancy MQTT broker adopting Serverless architecture
https://bifromq.io
Apache License 2.0
614 stars 61 forks source link

Setting Provider plugin value invalid #74

Closed lizhongren031 closed 4 months ago

lizhongren031 commented 5 months ago

Describe the bug We are set Setting Provider plugin code @Extension @Slf4j public class SettingProvider implements ISettingProvider {

@Override
public <R> R provide(Setting setting, String tenantId) {
    if (setting == Setting.DebugModeEnabled) {
        Boolean r = checkDebugMode(tenantId);
        log.info("Debug mode for tenant {} is {}", tenantId, r);
        return (R) r;
    }else if(setting==Setting.MQTT3Enabled){
        log.info("MQTT3Enabled for tenant {} is {}", tenantId, false);
        return (R) Boolean.valueOf(false);

    }
    return setting.current(tenantId);
}

} setting MQTT3Enabled for false ,than use client protocol 3 connect success

Debug code SettingProviderManager.java

public R provide(Setting setting, String tenantId) { assert !stopped.get(); R current = setting.current(tenantId); try { Timer.Sample sample = Timer.start(); R newVal = provider.provide(setting, tenantId); sample.stop(provideCallTimer); if (setting.isValid(newVal)) { // update the value setting.current(tenantId, newVal); } else { log.warn("Invalid setting value: setting={}, value={}", setting.name(), newVal); } } catch (Throwable e) { log.error("Setting provider throws exception: setting={}", setting.name(), e); // keep current value in case provider throws setting.current(tenantId, current); provideCallErrorCounter.increment(); } return current; }

newVal is my Setting Provider value for false is right,but last return current is default value to true. Whether to return newVal?

public TenantSettings(String tenantId, ISettingProvider provider) { mqtt3Enabled = provider.provide(MQTT3Enabled, tenantId); mqtt4Enabled = provider.provide(MQTT4Enabled, tenantId); mqtt5Enabled = provider.provide(MQTT5Enabled, tenantId); }

JVM:

BifroMQ

Additional context Add any other context about the problem here.

popduke commented 4 months ago

Thanks for your report. The fix has been included in v3.0.1. Additionally, the documentation has been updated to reflect the caching behavior for Tenant Settings.