search

apache / drill

Apache Drill is a distributed MPP query layer for self describing data
https://drill.apache.org/
Apache License 2.0
1.93k stars 984 forks source link

DRILL-8468: Drill doesn't perform drill.exec.storage.action_on_plugins_override_file action #2859

Closed rymarm closed 6 months ago

rymarm commented 7 months ago

DRILL-8468: Drill doesn't perform drill.exec.storage.action_on_plugins_override_file action

Description

The issue is caused by plugin registry refactoring: https://github.com/apache/drill/pull/1988 After this change, drill.exec.storage.action_on_plugins_override_file doesn't work as described in our documentation: https://drill.apache.org/docs/configuring-storage-plugins/#configuring-storage-plugins-with-the-storage-plugins-overrideconf-file

storage-plugins-override.conf is neither removed nor renamed on Drill restart.

Documentation

No updates

Testing

Manual

jnturton commented 7 months ago

Rebasing should fix the CI here too.

rymarm commented 6 months ago

Are the two removed null checks not needed?

@jnturton Yes, they are redundant. 

        if (locatorPlugins != null) {
          bootstrapPlugins.putAll(locatorPlugins);
        }

It is redundant, because StoragePlugins#putAll himself checks the argument nullability: https://github.com/apache/drill/blob/15c446d7e8ebe6d97fc879103e1710d733318c6d/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/StoragePlugins.java#L114-L117

      if (upgraded != null) {
        upgraded.putAll(locatorPlugins);
      }

It is redundant, because upgraded is just initialized with a constructor just a few lines above and not overridden.

jnturton commented 6 months ago

@rymarm okay let's rebase, get clean runs and merge!