[ 
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]

Reply via email to