SOLR-15986: CommitUpdateCommand and SplitIndexCommand can write user commit metadata. #608
Conversation
| @@ -54,6 +57,7 @@ public String toString() { | |||
| +",expungeDeletes="+expungeDeletes | |||
| +",softCommit="+softCommit | |||
| +",prepareCommit="+prepareCommit | |||
| +",commitData="+commitData | |||
There was a problem hiding this 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.
There was a problem hiding this comment.
I'll add only if commitData is not null.
| 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) { |
There was a problem hiding this 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.
There was a problem hiding this comment.
I would prefer to keep the building here as we always want to add this Solr-internal data. As I see it, CommitUpdateCommand is the user command, it could be error-prone to have these internal params in CommitUpdateCommand.
| @@ -176,14 +176,18 @@ private SolrIndexWriter(SolrCore core, String name, String path, Directory direc | |||
| } | |||
| } | |||
|
|
|||
| public static void setCommitData(IndexWriter iw, long commitCommandVersion) { | |||
There was a problem hiding this 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?
There was a problem hiding this comment.
The call from DirectUpdateHandler2.closeWriter() can provide a null param.
There was a problem hiding this comment.
The call from SolrIndexSplitter.doSplit() could set the commit metadata from the split command itself because it commits. I'll update this PR to also allow the (plugin writer) user to set commit metadata in the split command.
|
Yes I'm thinking of it just as an internal feature. |
bruno-roustant
changed the title
https://issues.apache.org/jira/browse/SOLR-15986