You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/11/23 08:30:26 UTC
[GitHub] [lucene] jpountz opened a new pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
jpountz opened a new pull request #466:
URL: https://github.com/apache/lucene/pull/466
In order for tests to keep running fast, this annotates all tests of N-2 codecs
with `@Nightly`. To keep good coverage of releases, the smoke tester is now
configured to run nightly tests.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976542454
I'm used to three levels - all regular tests (for local checks prior to commit), Slow (these run on a CI push) and Nightly (require more resources and/or time). So Slow tests are typically turned off and left for the CI run to verify... which is fine most of the time if it's a pull-request or a branch commit, for example.
But you're right that simpler is usually better and those test groups (other than badapple/awaitsfix) weren't that popular/ used anyway.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976527314
@dweiss I considered dedicated test group as well, but I like the `@Nightly` solution that keeps it simple and doesn't add more test groups :).
In fact, we may want to look into GC'ing test groups such as `@BadApple` that lucene is not using. I am also suspicious of the value we get from `@Slow`: they are enabled by default, hence such tests run everywhere by default (PR checks, ordinary jenkins builds), so personally i'd never risk turning them off only to see a code change immediately fail.
On the other hand, `@Nightly` seems appropriate and useful for this situation, most code changes honestly don't impact the index format. If you are making an index format change, it is typically a significant effort anyway, no big deal to just turn on nightly too before making a PR, etc.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976562717
It'd be good to periodically have some kind of sanity check for nightly test runs too as they tend to become a dark, throw-it-under-the-rug place for long-running junk. Even nightly runs should have a reasonable end horizon to complete... We do have the default suite timeout (adjusted on some monster tests) for this but it'd be great to have an over-time trend in the spirit of what Mike has for regression-testing.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976549766
I think for most lucene tests currently marked `@Slow`, there are so many paths to fix them to no longer need it:
* Run the test with `tests.profile` and actually fix something slow about it.
* Tone down test's iterations and rely on the "multipliers" to crank it up in jenkins.
* Split test into two methods, a smaller method with less iterations, and a separate bigger Nightly method.
* If it is really just exceptionally crazy slow and nothing can be "toned down", mark it Nightly.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976572901
Interesting, I agree with that. I personally really do try to avoid just "tossing tests under the rug" to Nightly when working on the tests performance. To me it is the last resort, they are a necessary? evil. I'd rather that we have fast tests organized in a simple system that everyone understands, e.g. no surprises where a PR build fails but things passed for the contributor locally, no surprises where a nightly build fails after a PR build succeeded, etc.
Often times it is just so easy to fix the tests in another way: less iterations, less threads (only max 2 should ever be needed IMO), less documents, etc etc.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a change in pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #466:
URL: https://github.com/apache/lucene/pull/466#discussion_r754892231
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene70/IndexedDISI.java
##########
@@ -1,326 +0,0 @@
-/*
- * 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.lucene.backward_codecs.lucene70;
-
-import java.io.DataInput;
-import java.io.IOException;
-import org.apache.lucene.search.DocIdSetIterator;
-import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.store.IndexOutput;
-import org.apache.lucene.util.BitSetIterator;
-import org.apache.lucene.util.FixedBitSet;
-import org.apache.lucene.util.RoaringDocIdSet;
-
-/**
- * Disk-based implementation of a {@link DocIdSetIterator} which can return the index of the current
- * document, i.e. the ordinal of the current document among the list of documents that this iterator
- * can return. This is useful to implement sparse doc values by only having to encode values for
- * documents that actually have a value.
- *
- * <p>Implementation-wise, this {@link DocIdSetIterator} is inspired of {@link RoaringDocIdSet
- * roaring bitmaps} and encodes ranges of {@code 65536} documents independently and picks between 3
- * encodings depending on the density of the range:
- *
- * <ul>
- * <li>{@code ALL} if the range contains 65536 documents exactly,
- * <li>{@code DENSE} if the range contains 4096 documents or more; in that case documents are
- * stored in a bit set,
- * <li>{@code SPARSE} otherwise, and the lower 16 bits of the doc IDs are stored in a {@link
- * DataInput#readShort() short}.
- * </ul>
- *
- * <p>Only ranges that contain at least one value are encoded.
- *
- * <p>This implementation uses 6 bytes per document in the worst-case, which happens in the case
- * that all ranges contain exactly one document.
- *
- * @lucene.internal
- */
-final class IndexedDISI extends DocIdSetIterator {
Review comment:
This was dead code, hence the removal.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #466:
URL: https://github.com/apache/lucene/pull/466#issuecomment-976503128
For what it's worth - we could also make it a dedicated test group (@@Backcompat)... don't know if it makes sense in any kind of isolation from nightly runs though, so perhaps not worth making life more complex.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz merged pull request #466: LUCENE-10168: Only test N-2 codecs on nightly runs.
Posted by GitBox <gi...@apache.org>.
jpountz merged pull request #466:
URL: https://github.com/apache/lucene/pull/466
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org