indexing-service no longer depends on indexing-hadoop#18247
indexing-service no longer depends on indexing-hadoop#18247gianm merged 10 commits intoapache:masterfrom
Conversation
…ng-hadoop depends on indexing-service
github-actions
bot
added
Area - Batch Ingestion
Area - Dependencies
Area - Ingestion
labels
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| private final IndexSpec indexSpec = IndexSpec.DEFAULT; | ||
|
|
||
| @Rule | ||
| public ExpectedException thrown = ExpectedException.none(); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| DataSchema.builder() | ||
| .withDataSource("foo") | ||
| .withGranularity( | ||
| new UniformGranularitySpec( | ||
| Granularities.DAY, | ||
| null, | ||
| ImmutableList.of(Intervals.of("2010-01-01/P1D")) | ||
| ) | ||
| ) | ||
| .withObjectMapper(jsonMapper) |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| DataSchema.builder() | ||
| .withDataSource("foo") | ||
| .withGranularity( | ||
| new UniformGranularitySpec( | ||
| Granularities.DAY, | ||
| null, ImmutableList.of(Intervals.of("2010-01-01/P1D")) | ||
| ) | ||
| ) | ||
| .withObjectMapper(jsonMapper) |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| rm -rf /var/cache/yum zulu-repo_${ZULU_REPO_VER}.noarch.rpm | ||
|
|
||
| ENV JAVA_HOME=/usr/lib/jvm/zulu17 | ||
| ENV JAVA_HOME=/usr/lib/jvm/zulu11 |
There was a problem hiding this comment.
this is the hadoop dockerfile, it doesn't support java 17
| @@ -162,7 +157,6 @@ private TaskConfig( | |||
| String baseDir, | |||
| File baseTaskDir, | |||
| String hadoopWorkingPath, | |||
There was a problem hiding this comment.
Should defaultHadoopCoordinates and hadoopWorkingPath be removed too?
| final SegmentMetadataPublisher maybeHandler; | ||
| if (config.isUpdaterJobSpecSet()) { | ||
| maybeHandler = new SegmentMetadataPublisher(INJECTOR.getInstance(IndexerMetadataStorageCoordinator.class)); | ||
| maybeHandler = new SegmentMetadataPublisher(HadoopTask.INJECTOR.getInstance(IndexerMetadataStorageCoordinator.class)); |
There was a problem hiding this comment.
Just saw that HadoopTask has its own static INJECTOR and EXTENSION_LOADER. 😅
Didn't go through the whole thing but seems like a really hacky approach to load a bunch of classes.
There was a problem hiding this comment.
yea, this isn't new to this PR 😅. i suppose this line change isn't really needed and was just a side-effect of moving stuff around, but will leave it unless i need to push more commits to this branch
…epends-on-indexing-service
…epends-on-indexing-service
gianm
left a comment
gianm
left a comment
There was a problem hiding this comment.
Couple of questions:
- Does the injector and classpath dark magic still work properly with this change? I recall there is some gnarly stuff where we're inspecting the classloader, pulling the jars out of it, and using that to populate the Hadoop classpath. (If the Hadoop ITs pass, probably the answer is yes this still works)
- With this change, if someone removes the
druid-hadoop-indexingjar, does Druid still work properly? If so that is great.
Yes, as far as I can tell everything is still working correctly. The Hadoop ITs don't actually pass for me because the query expectations it is doing afterwards are failing, but the ingestion tasks succeed (and i see the same query expectation failures before this change too, so I think it ok).
I haven't actually tried this yet, but I think it should be possible after #18246, since the only remaining references are the |
* indexing-service no longer depends on indexing-hadoop, instead indexing-hadoop depends on indexing-service * fixes * cleanup * fix it * fix declared dependencies * suppress warning * try again * suppress check
4 participants
Description
This PR makes it so that the
indexing-hadoopmodule is no longer a dependency ofindexing-service. Instead,indexing-hadoopdepends onindexing-service, and the only references toindexing-hadoopis theservicesmodule.To validate the changes I ran
ITHadoopIndexTest, which .. failed because I think this thing hasn't been updated in some time (i tried but gave up), but all of the hadoop ingestion tasks it ran as part of the tests ran and succeeded, just the queries after failed. I saw the same failures without the changes in this PR too (with the dockerfile and json file fixes from this PR), so I think everything is still ok.The next step to explore after this PR is moving the
indexing-hadoopmodule into a contrib extension.