[
https://issues.apache.org/jira/browse/SOLR-5944?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15300541#comment-15300541
]
Hoss Man commented on SOLR-5944:
--------------------------------
I'm still very slowly trying to get up to speed on this ... i started out by
reviewing the tests ishan wrote specifically for this issue, but once i
realized there really weren't any pre-existing, non trivial, "distributed
atomic updates" I put that on the backburner to work on SOLR-9159 -- now that
that test is solid and running on master & 6x, it's helped uncover an NPE in
the new code when the latest patch is applied...
{noformat}
[junit4] 2> 20685 ERROR (qtp1751829478-197) [n:127.0.0.1:58791_solr
c:test_col s:shard1 r:core_node4 x:test_col_shard1_replica2]
o.a.s.h.RequestHandlerBase org.apache.solr.common.SolrException: Exception
writing document id 34 to the index; possible analysis error.
[junit4] 2> at
org.apache.solr.update.DirectUpdateHandler2.addDoc(DirectUpdateHandler2.java:188)
[junit4] 2> at
org.apache.solr.update.processor.RunUpdateProcessor.processAdd(RunUpdateProcessorFactory.java:68)
[junit4] 2> at
org.apache.solr.update.processor.UpdateRequestProcessor.processAdd(UpdateRequestProcessor.java:48)
[junit4] 2> at
org.apache.solr.update.processor.DistributedUpdateProcessor.doLocalAdd(DistributedUpdateProcessor.java:954)
[junit4] 2> at
org.apache.solr.update.processor.DistributedUpdateProcessor.versionAdd(DistributedUpdateProcessor.java:1145)
[junit4] 2> at
org.apache.solr.update.processor.DistributedUpdateProcessor.processAdd(DistributedUpdateProcessor.java:729)
[junit4] 2> at
org.apache.solr.update.processor.LogUpdateProcessorFactory$LogUpdateProcessor.processAdd(LogUpdateProcessorFactory.java:103)
[junit4] 2> at
org.apache.solr.handler.loader.JavabinLoader$1.update(JavabinLoader.java:98)
[junit4] 2> at
org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$1.readOuterMostDocIterator(JavaBinUpdateRequestCodec.java:179)
[junit4] 2> at
org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$1.readIterator(JavaBinUpdateRequestCodec.java:135)
[junit4] 2> at
org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:274)
[junit4] 2> at
org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec$1.readNamedList(JavaBinUpdateRequestCodec.java:121)
[junit4] 2> at
org.apache.solr.common.util.JavaBinCodec.readVal(JavaBinCodec.java:239)
[junit4] 2> at
org.apache.solr.common.util.JavaBinCodec.unmarshal(JavaBinCodec.java:157)
[junit4] 2> at
org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec.unmarshal(JavaBinUpdateRequestCodec.java:186)
[junit4] 2> at
org.apache.solr.handler.loader.JavabinLoader.parseAndLoadDocs(JavabinLoader.java:108)
[junit4] 2> at
org.apache.solr.handler.loader.JavabinLoader.load(JavabinLoader.java:55)
[junit4] 2> at
org.apache.solr.handler.UpdateRequestHandler$1.load(UpdateRequestHandler.java:97)
[junit4] 2> at
org.apache.solr.handler.ContentStreamHandlerBase.handleRequestBody(ContentStreamHandlerBase.java:69)
[junit4] 2> at
org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:155)
[junit4] 2> at
org.apache.solr.core.SolrCore.execute(SolrCore.java:2036)
[junit4] 2> at
org.apache.solr.servlet.HttpSolrCall.execute(HttpSolrCall.java:658)
[junit4] 2> at
org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:465)
[junit4] 2> at
org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:257)
[junit4] 2> at
org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:208)
[junit4] 2> at
org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1676)
[junit4] 2> at
org.apache.solr.client.solrj.embedded.JettySolrRunner$DebugFilter.doFilter(JettySolrRunner.java:138)
[junit4] 2> at
org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1676)
[junit4] 2> at
org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:581)
[junit4] 2> at
org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:224)
[junit4] 2> at
org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1160)
[junit4] 2> at
org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:511)
[junit4] 2> at
org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
[junit4] 2> at
org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1092)
[junit4] 2> at
org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
[junit4] 2> at
org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:399)
[junit4] 2> at
org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
[junit4] 2> at
org.eclipse.jetty.server.Server.handle(Server.java:518)
[junit4] 2> at
org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:308)
[junit4] 2> at
org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:244)
[junit4] 2> at
org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:273)
[junit4] 2> at
org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:95)
[junit4] 2> at
org.eclipse.jetty.io.SelectChannelEndPoint$2.run(SelectChannelEndPoint.java:93)
[junit4] 2> at
org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.produceAndRun(ExecuteProduceConsume.java:246)
[junit4] 2> at
org.eclipse.jetty.util.thread.strategy.ExecuteProduceConsume.run(ExecuteProduceConsume.java:156)
[junit4] 2> at
org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:654)
[junit4] 2> at
org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:572)
[junit4] 2> at java.lang.Thread.run(Thread.java:745)
[junit4] 2> Caused by: java.lang.NullPointerException
[junit4] 2> at
org.apache.solr.update.DirectUpdateHandler2.doNormalUpdate(DirectUpdateHandler2.java:297)
[junit4] 2> at
org.apache.solr.update.DirectUpdateHandler2.addDoc0(DirectUpdateHandler2.java:221)
[junit4] 2> at
org.apache.solr.update.DirectUpdateHandler2.addDoc(DirectUpdateHandler2.java:176)
[junit4] 2> ... 47 more
...
[junit4] 2> NOTE: reproduce with: ant test
-Dtestcase=TestStressCloudBlindAtomicUpdates -Dtests.method=test_dv
-Dtests.seed=64C390303CA3F13A -Dtests.slow=true -Dtests.locale=el
-Dtests.timezone=Indian/Cocos -Dtests.asserts=true -Dtests.file.encoding=UTF-8
{noformat}
...that sort of failure has happened every time i've tried running that new
test with the current patch, so i don't think the seed matters.
----
I also realized i had some notes from lsat week when i first started reviewing
the tests that i aparently never posted here...
{panel}
Misc comments as i try to get up to wrap my head around some of these
tests/code...
* the ASL header should always be above the package & import statements
* we should do everything we can to avoid adding any more kitch sink schema
files -- if an existing schema file isn't good enough, include only what's
necessary to satisfy the test preconditions
** tests should also verify their schema preconditions to ensure no one edits
the schema to break assumptions (like docValues=true & stored=false)
*** schema API can be used to check this in cloud tests, or you can grab the
IndexSchema directly from the SolrCore in single core tests
* at first glance, SolrDocument.applyOlderUpdate looks like it's totally broken
for SolrInputDocument objects that contain multiple values for a single field
in the SolrInputDocument -- is that usecase tested? where?
** unit tests of this method in solrj independent of the usage in UpdateLog
seems really important
* these empty catch blogs are really bad, and overlook the possibility of
serious bugs (either in the current code, or possibly being introduced in the
future) by assuming any failures encountered here are the type of failure the
code expects, w/o actually verifying it...{code}
try {
addAndGetVersion(sdoc("id","20", "_version_", 123456, "ratings", map("inc",
1)), null);
fail();
} catch (Exception ex) {}
{code}...it should be something like...{code}
SolrException exected = expectThrows(SolrException.class, () -> {
addAndGetVersion(...)});
assertEquals(/* something about expected */, expected.getSomething());
{code}
* given that the whole point of this functionality is that the docValues are
suppose to be updated "in place" i'm really suprised at how little is done to
try and actaully assert that..
** InPlaceUpdateDistribTest checks the internal lucene "docid", but i don't see
similar checks in the other tests.
** it seems like every one of these tests should really be looking at the
segments in the index/replicas (via the Segments API for cloud, or more easily
by grabbing the IndexReader direct from the SolrCore in the single node test)
to really be absolutely sure that we didn't get a new segment unless we
expected one. That would not only help prevent false positives in the doc
values updating code, but could also help future proof if some
settings/defaults change in the future that cause auto commits or something
else to happen that this test doesn't expect.
*** or am i missunderstanding something about how the docvalue updating code
works? ... IIUC we can tell the diff between new segments and existing segments
whose docvalues have been updated, is my understanding incorrect?
** any assertions that are _not_ specific to checking docvalues are updated "in
place" could be refactored so that we can test them for *any* field, not just
pure DV fields.
*** for example: the exsiting bulkd of the test logic could be refactored into
a helper method that takes in a field name -- that helper method could then be
called from different test methods that pass in a hardcoded field name. the
test method that passes in a field name which is DV only, could have additional
asserts that the segment files on disk do not change after the test passes.
* it's hard to put a lot of faith in TestInPlaceUpdate and
InPlaceUpdateDistribTest considering they use so few docs that they aren't
likely to involve more then one index segment per shard (even in nightly or
with test.multiplier) ... using things like "atLeast()" to ensure we have a non
trivial number of docs in the index, and then indexing a random number of them
before the docs you are are actually testing against (and the rest after) would
help
** likewise for TestStressInPlaceUpdates ... use atLeast for the ndocs (and
maybe the num threads) so it's lower on basic runs, but higher for nightly runs
or runs where tests.multiplier is set
* is there a new test showing combinations/sequences of pure-dv updates
followed by atomic updates of multiple fields, followed by pure-dv updates? to
sanity check that (older style) atomic updates in the middle read the correct
"updated" docvalues?
{panel}
...the comment about refactoring the parts of the test that aren't specific to
"in place" DV updates is also applicable to the new
TestStressCloudBlindAtomicUpdates i added -- in that class "test_dv" could be
updated to assert that the segments on disk didn't change after the test is
finished.
> Support updates of numeric DocValues
> ------------------------------------
>
> Key: SOLR-5944
> URL: https://issues.apache.org/jira/browse/SOLR-5944
> Project: Solr
> Issue Type: New Feature
> Reporter: Ishan Chattopadhyaya
> Assignee: Shalin Shekhar Mangar
> Attachments: DUP.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch, SOLR-5944.patch,
> TestStressInPlaceUpdates.eb044ac71.beast-167-failure.stdout.txt,
> TestStressInPlaceUpdates.eb044ac71.beast-587-failure.stdout.txt,
> TestStressInPlaceUpdates.eb044ac71.failures.tar.gz,
> hoss.62D328FA1DEA57FD.fail.txt, hoss.62D328FA1DEA57FD.fail2.txt,
> hoss.62D328FA1DEA57FD.fail3.txt, hoss.D768DD9443A98DC.fail.txt,
> hoss.D768DD9443A98DC.pass.txt
>
>
> LUCENE-5189 introduced support for updates to numeric docvalues. It would be
> really nice to have Solr support this.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]