Gate analytics routing on cluster.pluggable.dataformat.enabled

[mch2]

This changes the routing gate to analytics-engine to use boolean flag with settings update consumer so it can be enabled/disabled.

Description

Gate analytics routing on cluster.pluggable.dataformat.enabled boolean.

Remove brittle string match on setting and make it match dynamic cluster level setting.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Race Condition The clusterDataformatEnabled field is declared volatile but is read and written without synchronization. While volatile ensures visibility, the constructor reads the initial value and then registers a consumer that updates it. If a cluster settings update occurs between reading the initial value (line 83-85) and registering the consumer (line 86-90), that update will be lost. This can happen during node startup when cluster state is being applied concurrently. this.clusterDataformatEnabled = IndicesService.CLUSTER_PLUGGABLE_DATAFORMAT_ENABLED_SETTING.get( clusterService.getSettings()); clusterService .getClusterSettings() .addSettingsUpdateConsumer( IndicesService.CLUSTER_PLUGGABLE_DATAFORMAT_ENABLED_SETTING, newValue -> this.clusterDataformatEnabled = newValue);

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Prevent potential memory leak The settings update consumer is registered but never unregistered, which could lead to memory leaks if the action instance is recreated. Consider implementing a cleanup method or using a weak reference pattern to prevent holding references to disposed instances. plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java [83-90] this.clusterDataformatEnabled = IndicesService.CLUSTER_PLUGGABLE_DATAFORMAT_ENABLED_SETTING.get( clusterService.getSettings()); +// Store consumer reference for potential cleanup +this.settingsConsumer = newValue -> this.clusterDataformatEnabled = newValue; clusterService .getClusterSettings() .addSettingsUpdateConsumer( IndicesService.CLUSTER_PLUGGABLE_DATAFORMAT_ENABLED_SETTING, - newValue -> this.clusterDataformatEnabled = newValue); + this.settingsConsumer); Suggestion importance[1-10]: 7 __ Why: This identifies a legitimate concern about the settings update consumer not being unregistered, which could cause memory leaks if RestUnifiedQueryAction instances are recreated. However, the suggested solution is incomplete as it doesn't show the cleanup method implementation or how settingsConsumer field would be declared.	Medium
	Use AtomicBoolean for thread safety The volatile keyword provides visibility guarantees but not atomicity. Since this field is written by the cluster-state applier thread and read by transport threads, consider using AtomicBoolean to ensure thread-safe updates and reads, especially if future changes might introduce compound operations. plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java [70] -private volatile boolean clusterDataformatEnabled; +private final AtomicBoolean clusterDataformatEnabled = new AtomicBoolean(); Suggestion importance[1-10]: 3 __ Why: While AtomicBoolean could provide additional safety, volatile is sufficient for simple read/write operations on a boolean field. The suggestion is technically valid but offers marginal improvement since the current implementation is correct for the use case described (single writer, multiple readers).	Low