SOLR-14660 - move HDFS to a module #324

warperwolf · 2021-10-01T19:33:36Z

https://issues.apache.org/jira/browse/SOLR-14660

Description

This PR moves HDFS from Core to a Contrib module.

Note: I kept separate commits to make it easier to review and to change / revert if necessary. They can be squashed once everything is polished and we have a green light.

Solution

A new contrib module has been created with its own build
HDFS classes have been moved to the new module
This PR does not yet take advantage of the Solr packaging system, this is just a separate module. Implementing as a package will be a next step.
HDFS directory direct references have been removed from Solr core (for example the UpdateHandler had a direct reference to HdfsDirectoryFactory, removed it so it works the same way as other directory factories and can also be loaded as a plugin (core.getResourceLoader().newInstance(ulogPluginInfo, UpdateLog.class, true);)
Solr Core still has some Hadoop-related (but non HDFS-related) classes (mostly Hadoop authentication), these are still left in Core , if they need to be separated, they will be handled in a separate jira
The block cache feature was only used by HDFS, moved to the contrib module)
The only change needed to use this module is to include its jar on the classpath (for example symlinking the jar to the web-inf/lib of the webapp), no changes to the solrconfig.xml are needed.
DirectoryFactory had DirectoryFactory.java: public final static String LOCK_TYPE_HDFS = "hdfs"; , but this was only referenced from the tests and from HdfsDirectoryFactory, removed it.
Removed the deprecated flags from the code and the warning from the ref guide.
Changed reference in ref guide to the new location of HdfsDirectoryFactory.
Added a simple readme markdown doc

Tests

Existing tests have been moved to the new contrib module where needed
Test have been refactored where a Hdfs test was extending a Core test class. For example, HdfsChaosMonkeyNothingIsSafeTest (hdfs test) extends ChaosMonkeyNothingIsSafeTest (core test).
Gradle does not allow dependencies between test projects of 2 different modules, so in such cases I introduced an abstract base class (here AbstractChaosMonkeyNothingIsSafeTest) which is extended by both. A good place to store these classes would be the test fixtures feature of gradle which is especially designed for this purpose, however some of the gradle plugins Solr uses fail because they are not compatible with fixtures (see https://issues.apache.org/jira/browse/SOLR-14660?focusedCommentId=17409690&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17409690 ), so I moved these classes to the Solr Test Framework project.
Solr core had some helper classes which are core related but they are also referenced by the hdfs classes (for example the MockCoreContainer.java) - moved those to gradle test fixtures of the Core project, and later moved them to the test framework
I also created a Cloudera CDP test release and built a cluster with HDFS and Solr and tested there
TestBackupRepositoryFactory had some references to HdfsBackupRepository, but the test case is not hdfs related. Removed that part.
HDFSRecoveryZKTest was also failing before this change. Added an AwaitsFix annotation, needs to be fixed separately.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide

This commit - moves the code implementing HDFS support to a contrib module which has its own build and tests. - moves block caching implementation to the HDFS contrib module - removes direct references from Solr Core to HDFS - moves test classes which are required both by Core and the HDFS contrib to the test framework - updates the documentation link to refer the HdfsDirectoryFactory from the new location - adds an AwaitsFix annotation to the HDFSRecoveryZKTest which was failing before these changes, needs to be fixed separately - removes the deprecated flags from HDFS classes - removes the deprecated notice from the reference guide After ensuring that the contrib module is on the classpath, HDFS support can be used as before, no changes need to be done to the configuration files. The contrib module is planned to be implemented as a Solr package in a next phase.

… the hdfs module

- refactored HdfsTestutil and created HadoopTestUtil

…epositoryFactory.java

…s (1.: HdfsChaosMonkeyNothingIsSafeTest) - HdfsChaosMonkeyNothingIsSafeTest was extending ChaosMonkeyNothingIsSafeTest.java, however since there is no depenency possible between test projects, it had to be split up - Created AbstractChaosMonkeyNotheingIsSafeTestBase as a common ancestor. Class is placed to the test framework because it has junit depenencies and needs to be shared - Moved the login in the test() method to doTest() which is now invoked from the child classes - Moved FullThrottleStoppableIndexingThread to the test framework as it needs to be shared - HdfsChaosMonkeyNothingIsSafeTest.java is moved to the hdfs plugin.

…s (2.: HdfsSyncSliceTest) - HdfsSyncSliceTest was extending SyncSliceTest

…s (3.: TestHdfsBackupRestoreCore) - TestHdfsBackupRestoreCore uses BackupRestoreUtils and BackupStatusChecker, these were moved to test framework (these also use log4j classes) - Moved the fetchRestoreStatus method out of TestRestoreCore to a util class as it is shared by hdfs tests and core tests

…s (4.: tests based on BasicDistributedZkTest) - HdfsNNFailoverTest extends BasicDistributedZkTest. Separated it to AbstractBasicDistributedZkTestBase and BasicDistributedZkTest. - Also changed the following classes to extend AbstractBasicDistributedZkTestBase: - HdfsBasicDistributedZkTest - HdfsWriteToMultipleCollectionsTest - StressHdfsTest

…s (5.: HdfsChaosMonkeySafeLeaderTest) - HdfsChaosMonkeySafeLeaderTest extends ChaosMonkeySafeLeaderTest - separated by introducing AbstractChaosMonkeySafeLeaderTestBase

…s (6.: HdfsBasicDistributedZk2Test) - HdfsBasicDistributedZk2Test extends BasicDistributedZk2Test - separated by introducing AbstractBasicDistributedZk2TestBase

…s (7.: HdfsRecoveryZkTest) - HdfsRecoveryZkTest extends RecoveryZkTest - introduced AbstractRecoveryZkTestBase

…s (8.: MoveReplicaTest) - MoveReplicaHDFSTest extends MoveReplicaTest - introduced AbstractMoveReplicaTestBase

…s (9.: RestartWhileUpdatingTest) - HDFSRestartWhileUpdatingTest extends RestartWhileUpdatingTest - introduced AbstractRestartWhileUpdatingTestBase

…s (10.: HdfsTlogReplayBufferedWhileIndexingTest) - HdfsTlogReplayBufferedWhileIndexingTest extends TlogReplayBufferedWhileIndexingTest - introduced AbstractTlogReplayBufferedWhileIndexingTestBase

…s (11.: HdfsUnloadDistributedZkTest) - HdfsUnloadDistributedZkTest extends UnloadDistributedZkTest - introduced AbstractUnloadDistributedZkTestBase

…s (12.: HdfsCollectionsAPIDistributedZkTest) - HdfsCollectionsAPIDistributedZkTest extends CollectionsAPIDistributedZkTest - introduced AbstractCollectionsAPIDistributedZkTestBase

…can be accessed by hdfs module tests

…d new test files

…fo and adjusted gradle validations to skip Hadoop code validations

Unfortunately it seems that some modules which are used by the gradle check are not compatible with test fixtures. This commit moves them to the test framework.

…endencies

- fixed a documentation link to point to hdfs contrib directory - added a short README.md on how to build and use - updated the solr on hdfs ref guide to remove deprecated flag

solr/contrib/hdfs/build.gradle

risdenk · 2022-01-28T17:01:50Z

I went through these two and I'm not sure there is a good way to address them right now. I don't think it should hold up the merge of this PR.

render-javadoc needs a little more attention - basically shouldn't need so many changes
- This is related to the bullet below. There were a few classes moved to test-framework which ended up in a few more split packages. This should be addressed for all of test-framework.
Abstract*Base test files moved to test-framework - not sure they are needed, but its a lot of the changes in this besides the move.
- This seems to be necessary due to how gradle and test dependencies end up interacting. It matches the existing pattern of some test-framework classes. The commit log has details about why each file was created/moved in test-framework.

solr/modules/hdfs/build.gradle

solr/core/build.gradle

madrob · 2022-01-28T20:55:08Z

I ran gradlew assemble and took a look at the output in solr/packaging/build... There were only 3 jars in modules/hdfs/lib/ - I'm not sure what I expected but for some reason I imagined there would be more?

Hadoop-auth, -annotations, and -common are all still in WEB-INF/lib, do we need all of those? I guess auth depends on common and we haven't moved that out yet?

madrob · 2022-01-28T21:17:27Z

I've been looking through and I think that we can exclude Hadoop-annotations and leave it in only as a test dependency? I'm not sure.

…tion instead of relative paths

risdenk · 2022-01-28T21:37:59Z

@madrob I'm with you I expected more jars to be there in hdfs/lib, but hadoop-auth, hadoop-annotations, and hadoop-common are all needed by the existing Hadoop authentication logic.

FWIW removing hadoop-annotations fails with:

> Task :solr:core:compileJava
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-common/3.2.0/e47a88c42c450e6e4b23bf951356c203cae2db24/hadoop-common-3.2.0.jar(/org/apache/hadoop/fs/FileSystem.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate': class file for org.apache.hadoop.classification.InterfaceAudience not found
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-common/3.2.0/e47a88c42c450e6e4b23bf951356c203cae2db24/hadoop-common-3.2.0.jar(/org/apache/hadoop/fs/FileSystem.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate'
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-common/3.2.0/e47a88c42c450e6e4b23bf951356c203cae2db24/hadoop-common-3.2.0.jar(/org/apache/hadoop/fs/Path.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate'
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-common/3.2.0/e47a88c42c450e6e4b23bf951356c203cae2db24/hadoop-common-3.2.0.jar(/org/apache/hadoop/security/UserGroupInformation.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate'
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-common/3.2.0/e47a88c42c450e6e4b23bf951356c203cae2db24/hadoop-common-3.2.0.jar(/org/apache/hadoop/security/UserGroupInformation.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate'
/Users/risdenk/.gradle/caches/modules-2/files-2.1/org.apache.hadoop/hadoop-auth/3.2.0/b1b95aed9aa956ffb7d21e30a0415ca14d91c4ad/hadoop-auth-3.2.0.jar(/org/apache/hadoop/security/authentication/util/KerberosName.class): warning: Cannot find annotation method 'value()' in type 'LimitedPrivate'
error: warnings found and -Werror specified

risdenk · 2022-01-28T22:12:24Z

Thanks @madrob and @dsmiley. I think your comments have been addressed by @warperwolf and I. Any other questions/comments/concerns/thoughts?

solr/core/src/java/org/apache/solr/update/UpdateHandler.java

This commit - moves the code implementing HDFS support to a new HDFS module which has its own build and tests. - moves block caching implementation to the HDFS module - removes direct references from Solr Core to HDFS module - moves test classes which are required both by Core and the HDFS module to the test framework - updates the documentation link to refer the HdfsDirectoryFactory from the new location - removes the deprecated flags from HDFS classes - removes the deprecated notice from the reference guide After ensuring that the module is on the classpath, HDFS support can be used as before, no changes need to be done to the configuration files. Some specific changes - Remove DirectoryFactory.LOCK_TYPE_HDFS - updateHandler will use the update log instance returned by the directory factory if no class is specified in solrconfig.xml. - test resource files are now copied from core upon build to avoid having to maintain 2 set of the same files in git. - HdfsCollectionsAPIDistributedZkTest extends CollectionsAPIDistributedZkTest - introduced AbstractCollectionsAPIDistributedZkTestBase - HdfsUnloadDistributedZkTest extends UnloadDistributedZkTest - introduced AbstractUnloadDistributedZkTestBase - HdfsTlogReplayBufferedWhileIndexingTest extends TlogReplayBufferedWhileIndexingTest - introduced AbstractTlogReplayBufferedWhileIndexingTestBase - HDFSRestartWhileUpdatingTest extends RestartWhileUpdatingTest - introduced AbstractRestartWhileUpdatingTestBase - MoveReplicaHDFSTest extends MoveReplicaTest - introduced AbstractMoveReplicaTestBase - HdfsRecoveryZkTest extends RecoveryZkTest - introduced AbstractRecoveryZkTestBase - HdfsBasicDistributedZk2Test extends BasicDistributedZk2Test - separated by introducing AbstractBasicDistributedZk2TestBase - HdfsChaosMonkeySafeLeaderTest extends ChaosMonkeySafeLeaderTest - separated by introducing AbstractChaosMonkeySafeLeaderTestBase - HdfsNNFailoverTest extends BasicDistributedZkTest. Separated it to AbstractBasicDistributedZkTestBase and BasicDistributedZkTest. - Also changed the following classes to extend AbstractBasicDistributedZkTestBase: - HdfsBasicDistributedZkTest - HdfsWriteToMultipleCollectionsTest - StressHdfsTest - TestHdfsBackupRestoreCore uses BackupRestoreUtils and BackupStatusChecker, these were moved to test framework (these also use log4j classes) - Moved the fetchRestoreStatus method out of TestRestoreCore to a util class as it is shared by hdfs tests and core tests - HdfsSyncSliceTest was extending SyncSliceTest - HdfsChaosMonkeyNothingIsSafeTest was extending ChaosMonkeyNothingIsSafeTest.java, however since there is no depenency possible between test projects, it had to be split up - Created AbstractChaosMonkeyNotheingIsSafeTestBase as a common ancestor. Class is placed to the test framework because it has junit depenencies and needs to be shared - Moved the login in the test() method to doTest() which is now invoked from the child classes - Moved FullThrottleStoppableIndexingThread to the test framework as it needs to be shared Closes PR #324 Co-authored-by: Istvan Farkas <ent128k@gmail.com> Co-authored-by: Kevin Risden <krisden@apache.org>

risdenk · 2022-01-31T20:50:54Z

Merged in cb2e58f

risdenk · 2022-01-31T20:52:28Z

Big shout out to @warperwolf who did 99% of the heavy lifting here.

warperwolf added 27 commits October 1, 2021 17:45

HDFS contrib module: moved the hdfs block directory implementation to…

edf037f

… the hdfs module

HDFS contrib module: removed HDFS direct reference from UpdateHandler

7d4903c

HDFS contrib module: moved shared core test classes to fixtures

ee32935

HDFS contrib module: separated HDFS vs Hadoop Test Classes

b640954

- refactored HdfsTestutil and created HadoopTestUtil

HDFS contrib module: removed HDFS class dependencies from TestBackupR…

a298d2b

…epositoryFactory.java

HDFS contrib module: separate HDFS test classes from Core test classe…

73b1fd8

…s (2.: HdfsSyncSliceTest) - HdfsSyncSliceTest was extending SyncSliceTest

HDFS contrib module: separate HDFS test classes from Core test classe…

7567da7

…s (5.: HdfsChaosMonkeySafeLeaderTest) - HdfsChaosMonkeySafeLeaderTest extends ChaosMonkeySafeLeaderTest - separated by introducing AbstractChaosMonkeySafeLeaderTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

508e053

…s (6.: HdfsBasicDistributedZk2Test) - HdfsBasicDistributedZk2Test extends BasicDistributedZk2Test - separated by introducing AbstractBasicDistributedZk2TestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

77f7be8

…s (7.: HdfsRecoveryZkTest) - HdfsRecoveryZkTest extends RecoveryZkTest - introduced AbstractRecoveryZkTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

e90b3a5

…s (8.: MoveReplicaTest) - MoveReplicaHDFSTest extends MoveReplicaTest - introduced AbstractMoveReplicaTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

b1128b7

…s (9.: RestartWhileUpdatingTest) - HDFSRestartWhileUpdatingTest extends RestartWhileUpdatingTest - introduced AbstractRestartWhileUpdatingTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

1bbfcbd

…s (10.: HdfsTlogReplayBufferedWhileIndexingTest) - HdfsTlogReplayBufferedWhileIndexingTest extends TlogReplayBufferedWhileIndexingTest - introduced AbstractTlogReplayBufferedWhileIndexingTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

1a19808

…s (11.: HdfsUnloadDistributedZkTest) - HdfsUnloadDistributedZkTest extends UnloadDistributedZkTest - introduced AbstractUnloadDistributedZkTestBase

HDFS contrib module: separate HDFS test classes from Core test classe…

dfbb0e7

…s (12.: HdfsCollectionsAPIDistributedZkTest) - HdfsCollectionsAPIDistributedZkTest extends CollectionsAPIDistributedZkTest - introduced AbstractCollectionsAPIDistributedZkTestBase

HDFS contrib module: moved Fake Group Mapping to test fixtures so it …

1d5ee1f

…can be accessed by hdfs module tests

HDFS contrib module: added missing dependencies to gradle build, adde…

f60f7a1

…d new test files

HDFS contrib module: fix renderJavaDoc errors

cee1e28

HDFS contrib module: removed deprecated flags

c0be894

HDFS contrib module: added AwaitsFix annotation to HdfsRecoveryZKTest

e12f5b1

HDFS Contrib module: Cleaned unused imports, added missing package-in…

99302c8

…fo and adjusted gradle validations to skip Hadoop code validations

HDFS contrib module: moved test fixtures to the test framework.

b21c9be

Unfortunately it seems that some modules which are used by the gradle check are not compatible with test fixtures. This commit moves them to the test framework.

HDFS Contrib module: fixed build to not include hadoop transitive dep…

cfe474f

…endencies

HDFS Contrib Module: documentation updates

383b456

- fixed a documentation link to point to hdfs contrib directory - added a short README.md on how to build and use - updated the solr on hdfs ref guide to remove deprecated flag

sonatype-lift bot reviewed Oct 1, 2021

View reviewed changes

solr/contrib/hdfs/build.gradle Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Oct 1, 2021

View reviewed changes

solr/contrib/hdfs/build.gradle Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Oct 1, 2021

View reviewed changes

solr/contrib/hdfs/build.gradle Outdated Show resolved Hide resolved

risdenk added 3 commits January 28, 2022 11:33

Make sure to consume the confDir in HdfsDirectoryFactory

32aaf75

Add comment about render-javadoc

856ca1c

Minor edits to documentation

bb216a7

Undo some test file changes to make diff smaller

e75d6ac

risdenk changed the title ~~SOLR-14660 - move HDFS to a contrib module~~ SOLR-14660 - move HDFS to a module Jan 28, 2022

risdenk approved these changes Jan 28, 2022

View reviewed changes

madrob reviewed Jan 28, 2022

View reviewed changes

solr/modules/hdfs/build.gradle Outdated Show resolved Hide resolved

solr/core/build.gradle Show resolved Hide resolved

HDFS module: use gradle variables to specify core test resources loca…

843fc71

…tion instead of relative paths

Cleanup more dependencies

ae67b14

risdenk requested a review from dsmiley January 28, 2022 22:10

HDFS module: updated changelog and solr on hdfs documentation

2a414d2

dsmiley approved these changes Jan 29, 2022

View reviewed changes

solr/core/src/java/org/apache/solr/update/UpdateHandler.java Show resolved Hide resolved

madrob approved these changes Jan 31, 2022

View reviewed changes

risdenk closed this Jan 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SOLR-14660 - move HDFS to a module #324

SOLR-14660 - move HDFS to a module #324

Uh oh!

warperwolf commented Oct 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

Uh oh!

Uh oh!

madrob commented Jan 28, 2022

Uh oh!

madrob commented Jan 28, 2022

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

Uh oh!

risdenk commented Jan 31, 2022

Uh oh!

risdenk commented Jan 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SOLR-14660 - move HDFS to a module #324

SOLR-14660 - move HDFS to a module #324

Uh oh!

Conversation

warperwolf commented Oct 1, 2021

Description

Solution

Tests

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

Uh oh!

Uh oh!

madrob commented Jan 28, 2022

Uh oh!

madrob commented Jan 28, 2022

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

risdenk commented Jan 28, 2022

Uh oh!

Uh oh!

risdenk commented Jan 31, 2022

Uh oh!

risdenk commented Jan 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants