You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "arunramani (via GitHub)" <gi...@apache.org> on 2024/03/15 20:43:55 UTC
[PR] MetricsModule: inject DataSourceTaskIdHolder early (druid)
arunramani opened a new pull request, #16140:
URL: https://github.com/apache/druid/pull/16140
### Description
This fixes an issue where MSQ ingestion failed if the first monitor in the list is `ServiceStatusMonitor`. Before this change, setting the `ServiceStatusMonitor` as the first monitor will fail with errors in `CliPeon` like below
```
1) Error in custom provider, java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `org.apache.druid.segment.virtual.ExpressionVirtualColumn`, problem: Unable to provision, see the following errors:
1) Problem parsing object at prefix[druid.lookup]: Cannot construct instance of `org.apache.druid.query.lookup.LookupListeningAnnouncerConfig`, problem: Unable to provision, see the following errors:
1) Error in custom provider, java.lang.IllegalStateException: This is a proxy used to support circular references. The object we're proxying is not constructed yet. Please wait until after injection has completed to use this object.
at org.apache.druid.cli.CliPeon$1.getDataSourceFromTask(CliPeon.java:344) (via modules: com.google.inject.util.Modules$OverrideModule -> com.google.inject.util.Modules$OverrideModule -> org.apache.druid.cli.CliPeon$1)
at org.apache.druid.cli.CliPeon$1.getDataSourceFromTask(CliPeon.java:344) (via modules: com.google.inject.util.Modules$OverrideModule -> com.google.inject.util.Modules$OverrideModule -> org.apache.druid.cli.CliPeon$1)
while locating java.lang.String annotated with @com.google.inject.name.Named(value="druidDataSource")
for field at org.apache.druid.server.metrics.DataSourceTaskIdHolder.dataSource(DataSourceTaskIdHolder.java:29)
at org.apache.druid.server.metrics.MetricsModule.configure(MetricsModule.java:92) (via modules: com.google.inject.util.Modules$OverrideModule -> com.google.inject.util.Modules$OverrideModule -> org.apache.druid.server.metrics.MetricsModule)
while locating org.apache.druid.server.metrics.DataSourceTaskIdHolder
Caused by: java.lang.IllegalStateException: This is a proxy used to support circular references. The object we're proxying is not constructed yet. Please wait until after injection has completed to use this object.
at com.google.common.base.Preconditions.checkState(Preconditions.java:512)
at com.google.inject.internal.DelegatingInvocationHandler.invoke(DelegatingInvocationHandler.java:36)
at com.sun.proxy.$Proxy103.getDataSource(Unknown Source)
at org.apache.druid.cli.CliPeon$1.getDataSourceFromTask(CliPeon.java:344)
at org.apache.druid.cli.CliPeon$1$$FastClassByGuice$$1ae344b1.invoke(<generated>)
at com.google.inject.internal.ProviderMethod$FastClassProviderMethod.doProvision(ProviderMethod.java:264)
at com.google.inject.internal.ProviderMethod$Factory.provision(ProviderMethod.java:401)
at com.google.inject.internal.ProviderMethod$Factory.get(ProviderMethod.java:376)
at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1092)
```
The problem is happening when the task file is deserialized in `CliPeon.readTask`. This is the sequence of events
* `CliPeon.readTask` uses Jackson to deserialize the task
* Jackson uses guice to find some injectable values. One of these is `DataSourceTaskIdHolder`
* To inject that value, guice decides to use the provider annotated by `CliPeon.getDataSourceFromTask` and `CliPeon.getTaskIDFromTask`.
This creates a circular ref since `readTask` is the provider for `Task` and `CliPeon.getDataSourceFromTask` and `CliPeon.getTaskIDFromTask` use `Task` as their inputs.
However if you set any other monitor first such as `JvmMonitor`, then the injector doesn’t use the providers from `CliPeon`. Jackson is able to bind both datasource and task ID from the task file itself.
For now triggering the binding of `DataSourceTaskIdHolder` before any of the monitors are loaded resolves the issue but this does not seem like the right fix. I have not been able to figure out what the correct fix is but I suspect it's a lot deeper than this one-liner.
##### Key changed/added classes in this PR
* `MetricsModule`
This PR has:
- [x] been self-reviewed.
- [ ] a release note entry in the PR description.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] MetricsModule: inject DataSourceTaskIdHolder early (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on PR #16140:
URL: https://github.com/apache/druid/pull/16140#issuecomment-2004812716
@arunramani Is it possible to add a test case of any sort for this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] MetricsModule: inject DataSourceTaskIdHolder early (druid)
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s merged PR #16140:
URL: https://github.com/apache/druid/pull/16140
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] MetricsModule: inject DataSourceTaskIdHolder early (druid)
Posted by "arunramani (via GitHub)" <gi...@apache.org>.
arunramani commented on PR #16140:
URL: https://github.com/apache/druid/pull/16140#issuecomment-2007968070
> @arunramani Is it possible to add a test case of any sort for this ?
I don't think there's any easy way to unit test this specifically. However the existing unit tests cover the injection working correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org