You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/22 20:05:48 UTC
[GitHub] [solr] risdenk opened a new pull request #688: SOLR-16040: Fix split packages in hdfs module
risdenk opened a new pull request #688:
URL: https://github.com/apache/solr/pull/688
https://issues.apache.org/jira/browse/SOLR-16040
Its broken down into multiple commits just as I worked through it. The only weird commit is ce7d256c89eca082a3ebe59595a9d61e098871bc where there were a bunch of changes needed to open up update and transaction classes to allow the new `hdfs` package to work. Everything before was package private so it worked in the same package.
Tested with:
```
./gradlew check
./gradlew test --tests "*hdfs*" --tests "*Hdfs*" --tests "*HDFS*" --tests "*Hadoop*" -Ptests.jvms=4 -Ptests.jvmargs=-XX:TieredStopAtLevel=1 -Ptests.nightly=true -Ptests.awaitsfix=false -Ptests.badapples=false -Ptests.file.encoding=UTF-8
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r813128520
##########
File path: solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc
##########
@@ -452,7 +452,7 @@ An example configuration using these properties can be found below:
[source,xml]
----
<backup>
- <repository name="hdfs" class="org.apache.solr.core.backup.repository.HdfsBackupRepository" default="false">
+ <repository name="hdfs" class="org.apache.solr.hdfs.backup.repository.HdfsBackupRepository" default="false">
Review comment:
Addressed in [89993fd](https://github.com/apache/solr/pull/688/commits/89993fd056d8cdd8f752805b80968f371f5f2d2c)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r812451717
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
The solrLibPath var is set like this
```bash
solrLibPath="${SOLR_LIB_PATH:-${scriptDir}/../../solr-webapp/webapp/WEB-INF/lib/*:${scriptDir}/../../lib/ext/*}"
```
I think `../../../modules/hdfs/lib/solr-hdfs*` is missing there?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r813069936
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
So its not just a compile time dependency on `Path` - if you read the code there are tons of references to HDFS and distcp. Basically for snapshots to work you need a shared filesystem and HDFS is one of the only ones that works semi easily. But yea agree about backup-restore and if snapshots still is needed.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1048283210
While you're at it, could you please rename pertinent tests that aren't using camel case for Hdfs like `HDFSCollectionsAPITest` ? Maybe non-test classes too. This is the standard I've seen most projects gravitate towards instead of it being haphazard.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r812446036
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
Do we need to add modules/hdfs/... to `solrLibPath` for this tool to work? Why did the SolrSnapshotsTool need to move to hdfs module in the first plae?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1048378622
@dsmiley fixed `Filesystem` in [f5249ad](https://github.com/apache/solr/pull/688/commits/f5249adf5ae5f284a86c9c8e1490eabcf9782711)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r813066227
##########
File path: solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc
##########
@@ -452,7 +452,7 @@ An example configuration using these properties can be found below:
[source,xml]
----
<backup>
- <repository name="hdfs" class="org.apache.solr.core.backup.repository.HdfsBackupRepository" default="false">
+ <repository name="hdfs" class="org.apache.solr.hdfs.backup.repository.HdfsBackupRepository" default="false">
Review comment:
We need to add this in the `major-changes-in-solr-9.adoc`
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
The only hdfs specific dependency I see is Path... Is the commons-cli a transative dependency there? In general I'm not sure how useful this tool is now that the backup-restore functionality is really good, but I guess that's for a separate ticket.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1049185891
@HoustonPutman the force push wasn't the issue. I didn't pull the branch before making local changes to add the `solr/CHANGES.txt`. The only change made through the UI was your documentation change.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk merged pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk merged pull request #688:
URL: https://github.com/apache/solr/pull/688
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1049184919
Did anything else get overridden in the force push?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r812501623
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
Addressed in feec4cbdf31583684d44d1fd78956c6533c28156
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r812448324
##########
File path: solr/server/scripts/cloud-scripts/snapshotscli.sh
##########
@@ -11,7 +11,7 @@ run_solr_snapshot_tool() {
log4j_config="file:${scriptDir}/../../resources/log4j2-console.xml"
fi
PATH=${JAVA_HOME}/bin:${PATH} ${JVM} ${ZKCLI_JVM_FLAGS} -Dlog4j.configurationFile=${log4j_config} \
- -classpath "${solrLibPath}" org.apache.solr.core.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
+ -classpath "${solrLibPath}" org.apache.solr.hdfs.snapshots.SolrSnapshotsTool "$@" 2> /dev/null
Review comment:
`SolrSnapshotsTool` has a bunch of HDFS specific dependencies in it. I looked at trying to separate it a little when moving HDFS to a module, but it wasn't simple. I didn't find any reference to this tool in the ref guide when I checked so I'm not sure how much it is actually used.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
risdenk commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1048313488
@dsmiley I think I fixed all the class names in 1f47ffe
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #688:
URL: https://github.com/apache/solr/pull/688#issuecomment-1049186363
Ahh ok, thanks for explaining 🙂
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a change in pull request #688: SOLR-16040: Fix split packages in hdfs module
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #688:
URL: https://github.com/apache/solr/pull/688#discussion_r813152671
##########
File path: solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
##########
@@ -125,6 +125,8 @@ This is only applicable for users returning information in JSON format, which is
* SOLR-14660: HDFS storage support has been moved to a module. Existing Solr configurations do not need any HDFS-related
changes, however the module needs to be installed - see the section xref:deployment-guide:solr-on-hdfs.adoc[].
+* SOLR-16040: If you are using the HDFS backup repository, you need to change the repository class to `org.apache.solr.hdfs.backup.repository.HdfsBackupRepository` - see the HDFS section in xref:deployment-guide:backup-restore.adoc[].
Review comment:
```suggestion
* SOLR-16040: If you are using the HDFS backup repository, you need to change the repository class to `org.apache.solr.hdfs.backup.repository.HdfsBackupRepository` - see the xref:deployment-guide:backup-restore.adoc#hdfsbackuprepository[HDFS Backup Repository] section.
```
##########
File path: solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
##########
@@ -125,6 +125,8 @@ This is only applicable for users returning information in JSON format, which is
* SOLR-14660: HDFS storage support has been moved to a module. Existing Solr configurations do not need any HDFS-related
changes, however the module needs to be installed - see the section xref:deployment-guide:solr-on-hdfs.adoc[].
+* SOLR-16040: If you are using the HDFS backup repository, you need to change the repository class to `org.apache.solr.hdfs.backup.repository.HdfsBackupRepository` - see the HDFS section in xref:deployment-guide:backup-restore.adoc[].
Review comment:
```suggestion
* SOLR-16040: If you are using the HDFS backup repository, you need to change the repository class to `org.apache.solr.hdfs.backup.repository.HdfsBackupRepository` - see the xref:deployment-guide:backup-restore.adoc#hdfsbackuprepository[HDFS Backup Repository] section.
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org