Re: Q: 9.x upgrade to hppc 0.9.1

2024-05-25 Thread Dawid Weiss
Hi Chris,

Since Elasticsearch is deployed as a module, then we need to update to hppc
> 0.9.1 [2], but unfortunately this is not straightforward. In fact, Ryan has
> a PR open [3] for the past 2 years without completion! The iteration order
> of some collection types in hppc 0.9.x [*] is tickling some inadvertent
> order dependencies in Elasticsearch. It may take some time to track these
> down and fix them.
>

I understand it's a pain if the order changes from run to run but I don't
see a way this can be avoided ([1] is the issue you mentioned on gh). Tests
(and code) shouldn't rely on map/set ordering, although I realize it may be
difficult to weed out in such a large codebase.

For what it's worth, the next version of HPPC will be a proper module (with
com.carrotsearch.hppc id). Would it change anything/ make it easier if I
renamed it to just 'hppc'?

I wonder if others may run into either or both of these issues, as we have
> in Elasticsearch, if we release 9.11 with this change?
>

That's why I wasn't entirely sold on having HPPC as the dependency from
Lucene when Bruno mentioned it recently - the jar/module hell will surface
sooner than later... Maybe it'd be a better idea to just copy what's needed
to the core jar and expose those packages to other Lucene modules (so that
there is no explicit dependency on HPPC at all)? Bruno copied a lot of
those classes already anyway - don't know how much of it is left to copy to
drop the dependency.

Dawid

[1] https://github.com/carrotsearch/hppc/issues/228
[2]
https://github.com/carrotsearch/hppc/commit/d569a8944091844c62349646f8eeaf35ebfb5ba6


>
> -Chris.
>
> [1] https://github.com/apache/lucene/pull/13392
> [2] https://github.com/elastic/elasticsearch/pull/109006
> [3] https://github.com/elastic/elasticsearch/pull/84168
>
> [*] HPPC-186: A different strategy has been implemented for collision
> avalanche avoidance. This results in removal of Scatter* maps and sets and
> their unification with their Hash* counterparts. This change should not
> affect any existing code unless it relied on static, specific ordering of
> keys. A side effect of this change is that key/value enumerators will
> return a different ordering of their container's values on each invocation.
> If your code relies on the order of values in associative arrays, it must
> order them after they are retrieved. (Bruno Roustant).
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
>


Re: Improve testing

2024-05-25 Thread Stefan Vodita
Some useful documentation on the gradlew commands:
https://github.com/apache/lucene/blob/main/help/workflow.txt



On Sat, 25 May 2024 at 19:38, Stefan Vodita  wrote:

> I'll add a step in between 1 and 2 that I often forget: ./gradlew tidy
> This refactors your code to the style the project uses, which we have
> checks for.
>
>
> On Sat, 25 May 2024 at 00:53, Michael Froh  wrote:
>
>> Is your new test uncommitted?
>>
>> The Gradle check will fail if you have uncommitted files, to avoid the
>> situation where it "works on my machine (because of a file that I forgot to
>> commit)".
>>
>> The rough workflow is:
>>
>> 1. Develop stuff (code and/or tests).
>> 2. Commit it.
>> 3. Gradle check.
>> 4. If Gradle check fails, then make changes and amend your commit. Go to
>> 3.
>>
>> Hope that helps,
>> Froh
>>
>>
>> On Fri, May 24, 2024 at 3:31 PM Chang Hank 
>> wrote:
>>
>>> After I added the new test case, I failed the ./gradlew check and it
>>> seems like the check failed because I added the new test case.
>>> Is there anything I need to do before executing ./gradlew check?
>>>
>>> Best,
>>> Hank
>>>
>>> > On May 24, 2024, at 12:53 PM, Chang Hank 
>>> wrote:
>>> >
>>> > Hi Robert,
>>> >
>>> > Thanks for your advice, will look into it!!
>>> >
>>> > Best,
>>> > Hank
>>> >> On May 24, 2024, at 12:46 PM, Robert Muir  wrote:
>>> >>
>>> >> On Fri, May 24, 2024 at 2:33 PM Chang Hank 
>>> wrote:
>>> >>>
>>> >>> Hi all,
>>> >>>
>>> >>> I want to improve the code coverage for Lucene, which package would
>>> you recommend testing to do so? Do we need more coverage in the Core
>>> package?
>>> >>>
>>> >>
>>> >> Hello,
>>> >>
>>> >> I'd recommend looking at the help/tests.txt file, you can generate the
>>> >> coverage report easily and find untested code:
>>> >>
>>> >> https://github.com/apache/lucene/blob/main/help/tests.txt#L193
>>> >>
>>> >> -
>>> >> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>>> >> For additional commands, e-mail: dev-h...@lucene.apache.org
>>> >>
>>> >
>>>
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>>
>>>


Re: Improve testing

2024-05-25 Thread Stefan Vodita
I'll add a step in between 1 and 2 that I often forget: ./gradlew tidy
This refactors your code to the style the project uses, which we have
checks for.


On Sat, 25 May 2024 at 00:53, Michael Froh  wrote:

> Is your new test uncommitted?
>
> The Gradle check will fail if you have uncommitted files, to avoid the
> situation where it "works on my machine (because of a file that I forgot to
> commit)".
>
> The rough workflow is:
>
> 1. Develop stuff (code and/or tests).
> 2. Commit it.
> 3. Gradle check.
> 4. If Gradle check fails, then make changes and amend your commit. Go to 3.
>
> Hope that helps,
> Froh
>
>
> On Fri, May 24, 2024 at 3:31 PM Chang Hank 
> wrote:
>
>> After I added the new test case, I failed the ./gradlew check and it
>> seems like the check failed because I added the new test case.
>> Is there anything I need to do before executing ./gradlew check?
>>
>> Best,
>> Hank
>>
>> > On May 24, 2024, at 12:53 PM, Chang Hank 
>> wrote:
>> >
>> > Hi Robert,
>> >
>> > Thanks for your advice, will look into it!!
>> >
>> > Best,
>> > Hank
>> >> On May 24, 2024, at 12:46 PM, Robert Muir  wrote:
>> >>
>> >> On Fri, May 24, 2024 at 2:33 PM Chang Hank 
>> wrote:
>> >>>
>> >>> Hi all,
>> >>>
>> >>> I want to improve the code coverage for Lucene, which package would
>> you recommend testing to do so? Do we need more coverage in the Core
>> package?
>> >>>
>> >>
>> >> Hello,
>> >>
>> >> I'd recommend looking at the help/tests.txt file, you can generate the
>> >> coverage report easily and find untested code:
>> >>
>> >> https://github.com/apache/lucene/blob/main/help/tests.txt#L193
>> >>
>> >> -
>> >> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> >> For additional commands, e-mail: dev-h...@lucene.apache.org
>> >>
>> >
>>
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>
>>


Q: 9.x upgrade to hppc 0.9.1

2024-05-25 Thread Chris Hegarty
Hi,

For awareness, I would to like to raise a potential issue that we’ve run into 
when testing Elasticsearch with the latest 9.x branch.

A recent change in 9.x [1] has introduced a dependency on hppc 0.9.1. Hppc has 
added an explicit automatic module name in its manifest, which effectively 
changes the auto module name from the plain hppc (derived from the jar file 
name) to com.carrotsearch.hppc. So one must use 0.9.1 ( or 0.9.0 ) if deployed 
as a module - otherwise the resolution of the `org.apache.lucene.join` module 
will fail.

Since Elasticsearch is deployed as a module, then we need to update to hppc 
0.9.1 [2], but unfortunately this is not straightforward. In fact, Ryan has a 
PR open [3] for the past 2 years without completion! The iteration order of 
some collection types in hppc 0.9.x [*] is tickling some inadvertent order 
dependencies in Elasticsearch. It may take some time to track these down and 
fix them.

I wonder if others may run into either or both of these issues, as we have in 
Elasticsearch, if we release 9.11 with this change?

-Chris.

[1] https://github.com/apache/lucene/pull/13392
[2] https://github.com/elastic/elasticsearch/pull/109006
[3] https://github.com/elastic/elasticsearch/pull/84168

[*] HPPC-186: A different strategy has been implemented for collision avalanche 
avoidance. This results in removal of Scatter* maps and sets and their 
unification with their Hash* counterparts. This change should not affect any 
existing code unless it relied on static, specific ordering of keys. A side 
effect of this change is that key/value enumerators will return a different 
ordering of their container's values on each invocation. If your code relies on 
the order of values in associative arrays, it must order them after they are 
retrieved. (Bruno Roustant).
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org