Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant merged PR #113: URL: https://github.com/apache/solr-sandbox/pull/113 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2647900256 I recently updated the Solr core dependency to 9.8, so I can complete this PR to fix the issue. The real fix is inside Solr core 9.8. Here I mainly add a test to verify that when nodes restart without commit, the encryption metadata in the commit metadata will still be transferred at the next commit after the restart. I could also fix EncryptionDirectory to support IndexFetcher copying the Lucene "segments" file (happens when the nodes restart). This file is not encrypted of course. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2427053387 I created a [PR](https://github.com/apache/solr/pull/2786) upstream to fix this. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2422961251 For the SolrIndexSplitter.doSplit, it is already handled in EncryptionUpdateHandler which extends DirectUpdateHandler2.split(). This method creates the SolrIndexSplitter and gives the Command which contains the metadata. For CoreContainer.reload, I missed it. Good point. I don't think there are other callers currently. But I agree this is fragile while Solr evolves. I hope that when this encryption module comes into Solr, we can encapsulate the Lucene IndexWriter in a way that it does the commit metadata transfer systematically at nearly the Lucene level. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
dsmiley commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2422926050 :embarrassed: totally an accident; sorry! I saw it was closed (didn't notice it was me!), so I deleted my comment to shift it to #109 I suppose IndexWriter.commit() is risky for index encryption... we must always guarantee there is commit metadata? This seems fragile; hard to guarantee, and with bad consequences. I did find-usages and I see two other callers, and would like your opinion: * CoreContainer.reload * SolrIndexSplitter.doSplit But finding all callers and adding protections, again, feels fragile. Like if we do index encryption, maybe we need a stronger lower level guarantee that the metadata is there. I haven't thought about this too much other than observing the fragility. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2422873354 @dsmiley why did you close this PR? It seems you wrote some comments, but I don't see them anymore. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
dsmiley commented on PR #113: URL: https://github.com/apache/solr-sandbox/pull/113#issuecomment-2422735985 I suppose IndexWriter.commit() is risky for index encryption... we must always guarantee there is commit metadata? It seems this is fragile right now. I did find-usages and I see two other callers, and would like your opinion: CoreContainer.reload SolrIndexSplitter.doSplit -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
dsmiley closed pull request #113: Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. URL: https://github.com/apache/solr-sandbox/pull/113 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on code in PR #113:
URL: https://github.com/apache/solr-sandbox/pull/113#discussion_r1806335514
##
encryption/src/main/java/org/apache/solr/update/DirectUpdateHandler2.java:
##
@@ -0,0 +1,1116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.update;
+
+import com.codahale.metrics.Meter;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Array;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.LongAdder;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
+import org.apache.lucene.index.CodecReader;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SlowCodecReaderWrapper;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.queries.function.ValueSource;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.BytesRefHash;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.SolrConfig.UpdateHandlerInfo;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean;
+import org.apache.solr.metrics.SolrDelegateRegistryMetricsContext;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.request.SolrRequestInfo;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.FunctionRangeQuery;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QueryUtils;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+import org.apache.solr.search.function.ValueSourceRangeFilter;
+import org.apache.solr.util.RefCounted;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * DirectUpdateHandler2 implements an UpdateHandler where
documents are added directly
+ * to the main Lucene index as opposed to adding to a separate smaller index.
+ */
+public class DirectUpdateHandler2 extends UpdateHandler
+implements SolrCoreState.IndexWriterCloser, SolrMetricProducer {
+
+ private static final int NO_FILE_SIZE_UPPER_BOUND_PLACEHOLDER = -1;
+
+ protected final SolrCoreState solrCoreState;
+
+ // stats
+ LongAdder addCommands = new LongAdder();
+ Meter addCommandsCumulative;
+ LongAdder deleteByIdCommands = new LongAdder();
+ Meter deleteByIdCommandsCumulative;
+ LongAdder deleteByQueryCommands = new LongAdder();
+ Meter deleteByQueryCommandsCumulative;
+ Meter expungeDeleteCommands;
+ Meter mergeIndexesCommands;
+ Meter commitCommands;
+ Meter splitCommands;
+ Meter optimizeCommands;
+ Meter rollbackCommands;
+ LongAdder numDocsPending = new LongAdder();
+ LongAdder numErrors = new LongAdder();
+ Meter numErrorsCumulative;
+
+ // tracks when auto-commit should occur
+ protected final CommitTracker commitTracker;
+ protected final CommitTracker softCommitTracker;
+
+ protected boolean commitWithinSoftCommit;
+
+ /**
+ * package access for testing
+ *
+ * @lucene.internal
+ */
+ void setCommitWithinSoftCommit(boolean value) {
+this.commitWithinSoftCommit = value;
+ }
+
+ private static final Logge
Re: [PR] Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called. [solr-sandbox]
bruno-roustant commented on code in PR #113:
URL: https://github.com/apache/solr-sandbox/pull/113#discussion_r1806335514
##
encryption/src/main/java/org/apache/solr/update/DirectUpdateHandler2.java:
##
@@ -0,0 +1,1116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.update;
+
+import com.codahale.metrics.Meter;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Array;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.LongAdder;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
+import org.apache.lucene.index.CodecReader;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.SlowCodecReaderWrapper;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.queries.function.ValueSource;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.BytesRefHash;
+import org.apache.solr.cloud.ZkController;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.SolrConfig.UpdateHandlerInfo;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrInfoBean;
+import org.apache.solr.metrics.SolrDelegateRegistryMetricsContext;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.request.LocalSolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.request.SolrRequestInfo;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.FunctionRangeQuery;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QueryUtils;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+import org.apache.solr.search.function.ValueSourceRangeFilter;
+import org.apache.solr.util.RefCounted;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * DirectUpdateHandler2 implements an UpdateHandler where
documents are added directly
+ * to the main Lucene index as opposed to adding to a separate smaller index.
+ */
+public class DirectUpdateHandler2 extends UpdateHandler
+implements SolrCoreState.IndexWriterCloser, SolrMetricProducer {
+
+ private static final int NO_FILE_SIZE_UPPER_BOUND_PLACEHOLDER = -1;
+
+ protected final SolrCoreState solrCoreState;
+
+ // stats
+ LongAdder addCommands = new LongAdder();
+ Meter addCommandsCumulative;
+ LongAdder deleteByIdCommands = new LongAdder();
+ Meter deleteByIdCommandsCumulative;
+ LongAdder deleteByQueryCommands = new LongAdder();
+ Meter deleteByQueryCommandsCumulative;
+ Meter expungeDeleteCommands;
+ Meter mergeIndexesCommands;
+ Meter commitCommands;
+ Meter splitCommands;
+ Meter optimizeCommands;
+ Meter rollbackCommands;
+ LongAdder numDocsPending = new LongAdder();
+ LongAdder numErrors = new LongAdder();
+ Meter numErrorsCumulative;
+
+ // tracks when auto-commit should occur
+ protected final CommitTracker commitTracker;
+ protected final CommitTracker softCommitTracker;
+
+ protected boolean commitWithinSoftCommit;
+
+ /**
+ * package access for testing
+ *
+ * @lucene.internal
+ */
+ void setCommitWithinSoftCommit(boolean value) {
+this.commitWithinSoftCommit = value;
+ }
+
+ private static final Logge
