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/08 15:47:24 UTC
[GitHub] [solr] dsmiley commented on a change in pull request #608: SOLR-15986: CommitUpdateCommand writes user commit metadata.
dsmiley commented on a change in pull request #608:
URL: https://github.com/apache/solr/pull/608#discussion_r801771033
##########
File path: solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java
##########
@@ -176,14 +176,18 @@ private SolrIndexWriter(SolrCore core, String name, String path, Directory direc
}
}
+ public static void setCommitData(IndexWriter iw, long commitCommandVersion) {
+ setCommitData(iw, commitCommandVersion, null);
+ }
+
@SuppressForbidden(reason = "Need currentTimeMillis, commit time should be used only for debugging purposes, " +
" but currently suspiciously used for replication as well")
- public static void setCommitData(IndexWriter iw, long commitCommandVersion) {
- log.debug("Calling setCommitData with IW:{} commitCommandVersion:{}", iw, commitCommandVersion);
- final Map<String,String> commitData = new HashMap<>();
- commitData.put(COMMIT_TIME_MSEC_KEY, String.valueOf(System.currentTimeMillis()));
- commitData.put(COMMIT_COMMAND_VERSION, String.valueOf(commitCommandVersion));
- iw.setLiveCommitData(commitData.entrySet());
+ public static void setCommitData(IndexWriter iw, long commitCommandVersion, Map<String, String> commitData) {
Review comment:
Perhaps building the whole map should actually go into CommitUpdateCommand? -- `getCommitMetadata()` and separately have `getCustomCommitMetadata` to differentiate the user provided add-on stuff from that provided by Solr itself? Just an idea.
##########
File path: solr/core/src/java/org/apache/solr/update/CommitUpdateCommand.java
##########
@@ -54,6 +57,7 @@ public String toString() {
+",expungeDeletes="+expungeDeletes
+",softCommit="+softCommit
+",prepareCommit="+prepareCommit
+ +",commitData="+commitData
Review comment:
instead please only do this if there is some data. This will be rare. I think this toString gets printed in logs and it's already rather verbose given some of these settings are more rare. If you are feeling slightly ambitious, maybe only add the "true" & non-null items but not others.
##########
File path: solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java
##########
@@ -176,14 +176,18 @@ private SolrIndexWriter(SolrCore core, String name, String path, Directory direc
}
}
+ public static void setCommitData(IndexWriter iw, long commitCommandVersion) {
Review comment:
It appears this is now unused (sorta) I'd just remove it. It's too internal to worry about back-compat.
I see one remaining caller; did you deliberately choose for that to not call the new version?
--
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