Iceberg writer stage should include WRITER_MAXIMUM_POOL_SIZE in its parameters

[liuml07]

Context

Iceberg writer stage missed to add WRITER_MAXIMUM_POOL_SIZE into the parameter list. So all overrides in jobs will be ignored and the default value will be used regardless.

Checklist

• ./gradlew build compiles code correctly
• Added new tests where applicable
• ./gradlew test passes all tests
• Extended README or added javadocs where applicable
• Added copyright headers for new files from CONTRIBUTING.md

[liuml07]

Trying to add a unit test but the patch 1) straightfoward 2) not easy to valid such coding omission. Perhaps we can add some test in spring-boot module after this is merged if necessary.

[calvin681]

You may want to back port this to the release-1.3 branch.

[sundargates]

@liuml07 It looks like we should be using Parameters::get without the default value in this case. That would surface the runtime exception and hence the problem in this case.

public Object get(String key) {
        // check if required parameter, make sure
        // has a value
        if (requiredParameters.contains(key) &&
                !state.containsKey(key)) {
            throw new ParameterException("Attempting to reference a required parameter witn no value: " + key + ", check parameter definitions for job.");
        }
        // check if parameter definition exists
        if (!parameterDefinitions.contains(key)) {
            throw new ParameterException("Attempting to reference parameter: " + key + ", with no definition, check parameter definitions in job.");
        }
        return state.get(key);
    }

    public Object get(String key, Object defaultValue) {
        Object value = defaultValue;
        try {
            value = get(key);
            if (value == null) {
                value = defaultValue;
            }
        } catch (ParameterException ex) {
            value = defaultValue;
        }
        return value;
    }

[codyrioux]

I agree with Sundaram in that we should not be using a default at the call site where we fetch the parameter.

[liuml07]

Makes sense. I'll file a follow-up PR to change all callsites in Iceberg sink module.