Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lucene index fixes #3142

Merged
merged 2 commits into from Jan 17, 2020
Merged

Conversation

@wolfgangmm
Copy link
Member

wolfgangmm commented Dec 5, 2019

In specific cases a wrong lucene index config (potentially belonging to a different collection) was used when storing a document into a collection which did not define a lucene index at all. We need to make sure the config is cleared and set to an empty default before the worker is reused for storing another document. The same applies to the range index.

@wolfgangmm wolfgangmm force-pushed the wolfgangmm:bugfix/lucene-facets-config branch from 800b092 to 0d61761 Jan 1, 2020
@wolfgangmm wolfgangmm changed the title WIP: Lucene index fixes Lucene index fixes Jan 13, 2020
@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 14, 2020

@wolfgangmm Could you please confirm if this PR is ready for review? I see that it is failing Travis and Appveyor.

@wolfgangmm

This comment has been minimized.

Copy link
Member Author

wolfgangmm commented Jan 14, 2020

@joewiz Yes, I was running this PR for a while. Obviously concurrency plays a role for the issue, so I wanted to be sure I did not miss anything. Seems all fine though.

@wolfgangmm

This comment has been minimized.

Copy link
Member Author

wolfgangmm commented Jan 14, 2020

@joewiz the CI failures were due to some maven download failing. I restarted travis.

wolfgangmm added 2 commits Dec 5, 2019
…ument into a collection which did not define a lucene index at all. LuceneIndexWorker in this case should reset its config property to an empty default config to avoid unwanted indexes being created.
@wolfgangmm wolfgangmm force-pushed the wolfgangmm:bugfix/lucene-facets-config branch from 0d61761 to e586fcb Jan 14, 2020
@joewiz
joewiz approved these changes Jan 14, 2020
Copy link
Member

joewiz left a comment

@wolfgangmm Thank you! I can confirm that with this PR, the revisions I contributed to the facets.xql xqsuite tests (avoid-range-index-conflict-city and avoid-range-index-conflict-person) now pass, with these results:

<testsuite package="http://exist-db.org/xquery/lucene/test/facets"
    timestamp="2020-01-14T17:29:59.016-05:00" tests="59" failures="0" errors="0" pending="0"
    time="PT0.265S">
    <testcase name="avoid-range-index-conflict-city" class="facet:avoid-range-index-conflict-city"/>
    <testcase name="avoid-range-index-conflict-person"
        class="facet:avoid-range-index-conflict-person"/>
    <testcase name="default-analyzer-no-diacritics" class="facet:default-analyzer-no-diacritics"/>
    <testcase name="field-respects-ignore" class="facet:field-respects-ignore"/>
    <testcase name="hierarchical-facets-query" class="facet:hierarchical-facets-query"/>
    <testcase name="hierarchical-facets-query" class="facet:hierarchical-facets-query"/>
    <testcase name="hierarchical-facets-query" class="facet:hierarchical-facets-query"/>
    <testcase name="hierarchical-facets-query" class="facet:hierarchical-facets-query"/>
    <testcase name="hierarchical-facets-query" class="facet:hierarchical-facets-query"/>
    <testcase name="hierarchical-facets-retrieve" class="facet:hierarchical-facets-retrieve"/>
    <testcase name="hierarchical-facets-retrieve" class="facet:hierarchical-facets-retrieve"/>
    <testcase name="hierarchical-place" class="facet:hierarchical-place"/>
    <testcase name="index-keys" class="facet:index-keys"/>
    <testcase name="multiple-indexes-with-same-facet" class="facet:multiple-indexes-with-same-facet"/>
    <testcase name="query-all-and-facets" class="facet:query-all-and-facets"/>
    <testcase name="query-all-and-non-existing-facet" class="facet:query-all-and-non-existing-facet"/>
    <testcase name="query-and-drill-down" class="facet:query-and-drill-down"/>
    <testcase name="query-and-drill-down" class="facet:query-and-drill-down"/>
    <testcase name="query-and-drill-down" class="facet:query-and-drill-down"/>
    <testcase name="query-and-sort" class="facet:query-and-sort"/>
    <testcase name="query-and-sort-by-date" class="facet:query-and-sort-by-date"/>
    <testcase name="query-and-sort-by-numeric" class="facet:query-and-sort-by-numeric"/>
    <testcase name="query-and-sort-by-numeric" class="facet:query-and-sort-by-numeric"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field" class="facet:query-field"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-no-expression" class="facet:query-field-no-expression"/>
    <testcase name="query-field-with-analyzer" class="facet:query-field-with-analyzer"/>
    <testcase name="query-field-with-analyzer" class="facet:query-field-with-analyzer"/>
    <testcase name="query-field-with-analyzer" class="facet:query-field-with-analyzer"/>
    <testcase name="query-field-with-analyzer" class="facet:query-field-with-analyzer"/>
    <testcase name="query-field-with-analyzer" class="facet:query-field-with-analyzer"/>
    <testcase name="query-field-with-condition" class="facet:query-field-with-condition"/>
    <testcase name="query-field-with-condition" class="facet:query-field-with-condition"/>
    <testcase name="query-field-with-condition" class="facet:query-field-with-condition"/>
    <testcase name="query-field-with-condition" class="facet:query-field-with-condition"/>
    <testcase name="query-field-with-condition" class="facet:query-field-with-condition"/>
    <testcase name="query-field-with-keyword-analyzer"
        class="facet:query-field-with-keyword-analyzer"/>
    <testcase name="query-no-default-index-but-facet" class="facet:query-no-default-index-but-facet"/>
    <testcase name="query-no-index" class="facet:query-no-index"/>
    <testcase name="query-no-index" class="facet:query-no-index"/>
    <testcase name="query-with-union-and-facets" class="facet:query-with-union-and-facets"/>
    <testcase name="query-with-union-and-facets" class="facet:query-with-union-and-facets"/>
    <testcase name="retrieve-multiple-fields" class="facet:retrieve-multiple-fields"/>
    <testcase name="retrieve-non-existant-field" class="facet:retrieve-non-existant-field"/>
    <testcase name="retrieve-not-stored" class="facet:retrieve-not-stored"/>
    <testcase name="store-and-remove" class="facet:store-and-remove"/>
    <testcase name="test-field-type" class="facet:test-field-type"/>
</testsuite>
@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 14, 2020

I see Travis has now passed, and all but one of the Appveyor environments passed. The one failure appears to have been a timeout. I just restarted it.

@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 15, 2020

So after restarting that build twice, it continues to fail with an apparent timeout (at 60 min). Looking at the log, the build seems stop for ~24 mins:

[00:00:37] Default locale: en_US, platform encoding: Cp1252
[00:00:37] OS name: "windows server 2016", version: "10.0", arch: "amd64", family: "windows"
[00:24:30] ANTLR Parser Generator   Version 2.7.7 (20060906)   1989-2005
[00:24:30] 
[00:29:14] mvn -T 2C -Ddocker=false test -B

Having only started maven at ~29 mins into the test run, maven takes 30 mins and finishes just before the 60 min timeout:

[00:59:22] [INFO] ------------------------------------------------------------------------
[00:59:22] [INFO] BUILD SUCCESS
[00:59:22] [INFO] ------------------------------------------------------------------------
[00:59:22] [INFO] Total time:  30:06 min (Wall Clock)
[00:59:22] [INFO] Finished at: 2020-01-15T01:53:10Z
[00:59:22] [INFO] ------------------------------------------------------------------------
[00:59:23] $wc = New-Object 'System.Net.WebClient'
[00:59:23] foreach ($file in Get-ChildItem -Path "$($env:APPVEYOR_BUILD_FOLDER)\exist-core\target\surefire-reports\" -Filter *.xml) {
[00:59:23]   $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $file.FullName)
[00:59:23] }

... at which point the build fails.

@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 15, 2020

In contrast, the other windows build completes these steps in only ~10 mins:

[00:01:25] Default locale: en_US, platform encoding: Cp1252
[00:01:25] OS name: "windows server 2016", version: "10.0", arch: "amd64", family: "windows"
[00:06:32] ANTLR Parser Generator   Version 2.7.7 (20060ANTLR Parser Generator   Version 2.7.7 (20060906)   1989-2005
[00:06:32] 
[00:10:36] mvn -T 2C -Ddocker=false test -B

... and the maven build still takes quite some time (43 mins, compared to 30 mins above) but completes in just over 56 mins:

[00:53:57] [INFO] ------------------------------------------------------------------------
[00:53:57] [INFO] BUILD SUCCESS
[00:53:57] [INFO] ------------------------------------------------------------------------
[00:53:57] [INFO] Total time:  43:18 min (Wall Clock)
[00:53:57] [INFO] Finished at: 2020-01-14T18:11:28Z
[00:53:57] [INFO] ------------------------------------------------------------------------
[00:53:58] $wc = New-Object 'System.Net.WebClient'
[00:53:58] foreach ($file in Get-ChildItem -Path "$($env:APPVEYOR_BUILD_FOLDER)\exist-core\target\surefire-reports\" -Filter *.xml) {
[00:53:58]   $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", $file.FullName)
[00:53:58] }
[00:53:58] 
[00:55:34] Updating build cache...
[00:55:34] 
[00:55:39] Cache 'C:\Users\appveyor\.m2' - Calculating checksum...
[00:56:00] Cache 'C:\Users\appveyor\.m2' - Zipping...
[00:56:01] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...1%
[00:56:01] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...10%
[00:56:02] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...20%
[00:56:03] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...30%
[00:56:04] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...40%
[00:56:05] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...50%
[00:56:06] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...60%
[00:56:06] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...70%
[00:56:07] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...80%
[00:56:08] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...90%
[00:56:08] Cache 'C:\Users\appveyor\.m2' - Uploading (362,779,334 bytes)...100%
[00:56:08] Cache 'C:\Users\appveyor\.m2' - Updated
[00:56:08] 
[00:56:08] Cache 'C:\maven' - Up to date
[00:56:08] Cache entry not found: C:\projects\exist\$HOME\.m2
[00:56:08] Build success
@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 17, 2020

I've just triggered another run of the long-running Appveyor build, since other PRs have somehow managed to pass since my previous attempts...

@duncdrum

This comment has been minimized.

Copy link
Contributor

duncdrum commented Jan 17, 2020

@joewiz i m ok with merging this. My guess the cache of the build might be corrupt. So when triggering a rerun, you might have to manually delete the cache in the Travis GUI.

@duncdrum duncdrum merged commit ad564d7 into eXist-db:develop Jan 17, 2020
3 checks passed
3 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@joewiz

This comment has been minimized.

Copy link
Member

joewiz commented Jan 17, 2020

@duncdrum Ah, I hadn't realized that the CI servers' caches could become corrupted, but I'm glad to know there's a workaround. I also agree that the PR was good to merge and that this was clearly a CI glitch. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.