CC-11984: Remove the table.whitelist and table.blacklist recommended values

[rhauch]

Problem

The JDBC source connector uses a TableRecommender for the table.whitelist and table.blacklist properties. This recommender uses the connector properties to establish a connection, and then read the accessible tables.

Unfortunately, AK's ConfigDef and AbstractConfig class only pass to each Recommender.validValues(...) methods the parsed configuration properties, which are only those properties that are defined with a ConfigDef.

In #870 we added the ability for users to define extra properties that would be passed to the JDBC driver when connecting to the database. These extra properties are not defined by ConfigDef as they vary by DBMS/driver, and therefore are not included when passed to any Recommender instances. In the case of the TableRecommender, if the extra properties included any properties required for connections (e.g., kerberos-related properties for the JDBC driver), then the recommender would fail to connect and would throw a ConnectException. The ConfigDef and AbstractConfig then record this as a configuration error on the property with the recommender (e.g., table.whitelist or table.blacklist).

IOW, if you use those extra connector.* properties to enable Kerberos, the connector would otherwise be able to connect except for the failing TableRecommender that does not see/use those connector.* properties.

One solution is to just change the TableRecommender to return an empty list if it could not connect, since any properties that prohibited connecting would have their own errors. Unfortunately, that results in a poor and inconsistent user experience, because sometimes the user would get recommended values in C3 for table.whitelist and table.blacklist, while other times they would get no recommended values. This behavior would not be very intuitive and would cause lots of headaches, since it would depend upon whether any connection.* properties were required to connect to the database and read the accessible tables.

Instead, the proposed solution is to simply remove TableRecommender altogether. This class has been a problem for quite some time:

1. If the database user has access to lots of tables, this recommender may take a long time to read the list of accessible tables, leading to long validation times or failed validations.
2. If the database user has access to lots of tables, the choice lists in C3 are super long and not really useable.
3. If the recommender returns large numbers of tables, the validation result can be very large, since these are once for table.whitelist and another time for table.blacklist.
4. The TableRecommender creates a new connection with every validation attempt for each of the two properties. This is exacerbated if the user is quickly trying different combinations of other properties. (Each connection is indeed closed via a try-with-resources block.)
5. Recommenders are always populated, but only used in tools like C3. Most other applications that validate a config with the REST API do not use the recommendations.

Since we're about to release the 10.0.0 version of the JDBC source and sink connectors, now would be a good time to change this behavior and remove this particular recommender. (The only other recommender in the JDBC source does not use a connection and is literally just a decision tree with other property values, so it’s worth keeping.)

Solution

Remove the TableRecommender to simplify the connector and eliminate potentially long-running operations. As a result, tools like C3 would no longer show recommended values for table.whitelist and table.blacklist properties.

Does this solution apply anywhere else?

• yes
• no

If yes, where?

Test Strategy

The test cases that verified the TableRecommender values were modified to assert that an empty list of tables is returned.

Testing done:

• Unit tests
• Integration tests
• System tests
• Manual tests

Release Plan

Include in the upcoming 10.0.0 release. Since there is no 10.0.x branch yet, this should be applied to the master branch.

[rhauch]

Closed #927 in favor of this approach.

[C0urante]

LGTM

[kkonstantine]

LGTM
Thanks @rhauch !