Re: Ban Java Streams usage in Ignite 3 code

2021-09-09 Thread Nikolay Izhikov
+1 to ban Streams usage.

 

> 9 сент. 2021 г., в 02:59, Valentin Kulichenko  
> написал(а):
> 
> Pavel,
> 
> Quite frankly, I think we used to lean into performance too much. We
> generally preferred it over data consistency, project modularity and code
> readability. Performance, of course, plays a very important rule in Ignite,
> but it's possible to overdo anything.
> 
> There are certainly parts of the project that can benefit from features
> like Stream API, without significant concern over performance. CLI is an
> obvious example, but I'm sure it's not the only one.
> 
> That said, I don't think that banning something is productive. At the same
> time, we should make sure we pay more attention to performance during
> reviews. Maybe we should have a checklist of major things to look for? Not
> as a set of strict rules, but more as a guideline for contributors and
> committers.
> 
> We might also want to start benchmarking the code and tracking the progress.
> 
> -Val
> 
> On Wed, Sep 8, 2021 at 12:09 PM Pavel Tupitsyn  wrote:
> 
>> Alexander, Ivan,
>> 
>>> not very productive to assume that 100% of your code is
>>> on the hot path
>> 
>> That is exactly what we should be doing.
>> When I joined Ignite community 6 years ago, this was the prevalent mindset.
>> 
>> I'm not sure which part of Ignite can be considered "not on a hot path".
>> Create/alter table (mentioned above) should perform well too.
>> 
>>> measured first and only then optimized
>>> https://wiki.c2.com/?OptimizeLater
>> 
>> Extra allocations are a "death by a thousand cuts".
>> They add up here and there, and then there are GC pauses.
>> This would be hard to "optimize later".
>> 
>> It is common for perf-oriented projects to avoid some techniques.
>> For example, LINQ (streams analog from C# with similar perf issues) is
>> avoided in libraries and compilers [1].
>> 
>> [1] https://github.com/dotnet/roslyn/blob/main/CONTRIBUTING.md
>> 
>> On Wed, Sep 8, 2021 at 9:49 PM Valentin Kulichenko <
>> valentin.kuliche...@gmail.com> wrote:
>> 
>>> I don't think we should ban anything. Streams API is just a tool in the
>>> toolbox - it should be used appropriately. It's up to the contributor and
>>> reviewer(s) to identify whether a particular usage might cause
>> performance
>>> issues.
>>> 
>>> -Val
>>> 
>>> On Wed, Sep 8, 2021 at 8:01 AM Alexander Polovtcev <
>>> alexpolovt...@gmail.com>
>>> wrote:
>>> 
 -1
 I think that it is not very productive to assume that 100% of your code
>>> is
 on the hot path, it would be impossible to write and maintain. Humans
>> are
 not very good at guessing where the performance bottlenecks are, so the
 performance of the possible hot paths should be measured first and only
 then optimized and documented.
 
 On Wed, Sep 8, 2021 at 5:53 PM Ivan Pavlukhin 
>>> wrote:
 
> Does not this trivial strategy work for us?
> https://wiki.c2.com/?OptimizeLater
> 
> 2021-09-08 13:52 GMT+03:00, Andrey Gura :
>> Agree that any additional object creation on a hot path could be a
>> problem. So hot path should not contain stream API and any other
>> potentially problem code (e.g. iterator instead of for by index).
>> 
>> On Wed, Sep 8, 2021 at 1:45 PM Pavel Tupitsyn <
>> ptupit...@apache.org>
> wrote:
>>> 
>>> Ok, maybe a total ban is overkill, but right now streams are used
>>> even
> on
>>> some hot paths like getAllAsync [1].
>>> 
>>> Another issue is that Collectors.toList() and other variants don't
> accept
>>> capacity, and we end up with unnecessary reallocations of
>> underlying
>>> arrays.
>>> 
>>> [1]
>>> 
> 
 
>>> 
>> https://github.com/apache/ignite-3/blob/1d7d703ff2b18234b15a9a7b03104fbb73388edf/modules/table/src/main/java/org/apache/ignite/internal/table/KVBinaryViewImpl.java#L83
>>> 
>>> On Wed, Sep 8, 2021 at 1:06 PM Konstantin Orlov <
>>> kor...@gridgain.com>
>>> wrote:
>>> 
 Hi!
 
 Agree with Ivan that it’s an overkill. Code readability and
 maintainability should have
 the same priority as the performance (with some exceptions of
 course).
 
 BTW the result of your benchmark looks quite strange. The
 performance
 penalty on
 my laptop (Core i7 9750H, 6 cores up to 4.50 GHz) is 25%, not 8
 times:
 
 Benchmark Mode  Cnt  Score Error
 Units
 JmhIncrementBenchmark.loopSumthrpt   10  32347.819 ± 676.548
 ops/ms
 JmhIncrementBenchmark.streamSum  thrpt   10  24459.196 ± 610.152
 ops/ms
 
 
 --
 Regards,
 Konstantin Orlov
 
 
> On 8 Sep 2021, at 12:23, Ivan Bessonov >> 
> wrote:
> 
> Hello Igniters,
> 
> I object, banning streams is an overkill. I would argue that
>>> most
 of
> the
>>

Re: Ban Java Streams usage in Ignite 3 code

2021-09-09 Thread Ivan Daschinsky
To be honest, Pavel, your benchmark is not quite correct. Please, rewrite
it using BlackHole

чт, 9 сент. 2021 г. в 10:28, Nikolay Izhikov :

> +1 to ban Streams usage.
>
>
>
> > 9 сент. 2021 г., в 02:59, Valentin Kulichenko <
> valentin.kuliche...@gmail.com> написал(а):
> >
> > Pavel,
> >
> > Quite frankly, I think we used to lean into performance too much. We
> > generally preferred it over data consistency, project modularity and code
> > readability. Performance, of course, plays a very important rule in
> Ignite,
> > but it's possible to overdo anything.
> >
> > There are certainly parts of the project that can benefit from features
> > like Stream API, without significant concern over performance. CLI is an
> > obvious example, but I'm sure it's not the only one.
> >
> > That said, I don't think that banning something is productive. At the
> same
> > time, we should make sure we pay more attention to performance during
> > reviews. Maybe we should have a checklist of major things to look for?
> Not
> > as a set of strict rules, but more as a guideline for contributors and
> > committers.
> >
> > We might also want to start benchmarking the code and tracking the
> progress.
> >
> > -Val
> >
> > On Wed, Sep 8, 2021 at 12:09 PM Pavel Tupitsyn 
> wrote:
> >
> >> Alexander, Ivan,
> >>
> >>> not very productive to assume that 100% of your code is
> >>> on the hot path
> >>
> >> That is exactly what we should be doing.
> >> When I joined Ignite community 6 years ago, this was the prevalent
> mindset.
> >>
> >> I'm not sure which part of Ignite can be considered "not on a hot path".
> >> Create/alter table (mentioned above) should perform well too.
> >>
> >>> measured first and only then optimized
> >>> https://wiki.c2.com/?OptimizeLater
> >>
> >> Extra allocations are a "death by a thousand cuts".
> >> They add up here and there, and then there are GC pauses.
> >> This would be hard to "optimize later".
> >>
> >> It is common for perf-oriented projects to avoid some techniques.
> >> For example, LINQ (streams analog from C# with similar perf issues) is
> >> avoided in libraries and compilers [1].
> >>
> >> [1] https://github.com/dotnet/roslyn/blob/main/CONTRIBUTING.md
> >>
> >> On Wed, Sep 8, 2021 at 9:49 PM Valentin Kulichenko <
> >> valentin.kuliche...@gmail.com> wrote:
> >>
> >>> I don't think we should ban anything. Streams API is just a tool in the
> >>> toolbox - it should be used appropriately. It's up to the contributor
> and
> >>> reviewer(s) to identify whether a particular usage might cause
> >> performance
> >>> issues.
> >>>
> >>> -Val
> >>>
> >>> On Wed, Sep 8, 2021 at 8:01 AM Alexander Polovtcev <
> >>> alexpolovt...@gmail.com>
> >>> wrote:
> >>>
>  -1
>  I think that it is not very productive to assume that 100% of your
> code
> >>> is
>  on the hot path, it would be impossible to write and maintain. Humans
> >> are
>  not very good at guessing where the performance bottlenecks are, so
> the
>  performance of the possible hot paths should be measured first and
> only
>  then optimized and documented.
> 
>  On Wed, Sep 8, 2021 at 5:53 PM Ivan Pavlukhin 
> >>> wrote:
> 
> > Does not this trivial strategy work for us?
> > https://wiki.c2.com/?OptimizeLater
> >
> > 2021-09-08 13:52 GMT+03:00, Andrey Gura :
> >> Agree that any additional object creation on a hot path could be a
> >> problem. So hot path should not contain stream API and any other
> >> potentially problem code (e.g. iterator instead of for by index).
> >>
> >> On Wed, Sep 8, 2021 at 1:45 PM Pavel Tupitsyn <
> >> ptupit...@apache.org>
> > wrote:
> >>>
> >>> Ok, maybe a total ban is overkill, but right now streams are used
> >>> even
> > on
> >>> some hot paths like getAllAsync [1].
> >>>
> >>> Another issue is that Collectors.toList() and other variants don't
> > accept
> >>> capacity, and we end up with unnecessary reallocations of
> >> underlying
> >>> arrays.
> >>>
> >>> [1]
> >>>
> >
> 
> >>>
> >>
> https://github.com/apache/ignite-3/blob/1d7d703ff2b18234b15a9a7b03104fbb73388edf/modules/table/src/main/java/org/apache/ignite/internal/table/KVBinaryViewImpl.java#L83
> >>>
> >>> On Wed, Sep 8, 2021 at 1:06 PM Konstantin Orlov <
> >>> kor...@gridgain.com>
> >>> wrote:
> >>>
>  Hi!
> 
>  Agree with Ivan that it’s an overkill. Code readability and
>  maintainability should have
>  the same priority as the performance (with some exceptions of
>  course).
> 
>  BTW the result of your benchmark looks quite strange. The
>  performance
>  penalty on
>  my laptop (Core i7 9750H, 6 cores up to 4.50 GHz) is 25%, not 8
>  times:
> 
>  Benchmark Mode  Cnt  Score Error
>  Units
>  JmhIncrementBenchmark.loopSumthrpt   10  32347.819 ± 676.548
> >>>

Re: Ban Java Streams usage in Ignite 3 code

2021-09-09 Thread Ivan Daschinsky
Few words about the topic.
There is a disease, quite common in the java community -- it is called the
streamosis.
But, to be honest, afear of streams is also not good.

As for me, sometimes rewriting code completely with simple loops often
makes it more readable, understandable and usually faster.

So I am against a complete ban of streams, but I am for using this tool
with caution. Often streams make code ugly and non-readable at all.


чт, 9 сент. 2021 г. в 10:50, Ivan Daschinsky :

> To be honest, Pavel, your benchmark is not quite correct. Please, rewrite
> it using BlackHole
>
> чт, 9 сент. 2021 г. в 10:28, Nikolay Izhikov :
>
>> +1 to ban Streams usage.
>>
>>
>>
>> > 9 сент. 2021 г., в 02:59, Valentin Kulichenko <
>> valentin.kuliche...@gmail.com> написал(а):
>> >
>> > Pavel,
>> >
>> > Quite frankly, I think we used to lean into performance too much. We
>> > generally preferred it over data consistency, project modularity and
>> code
>> > readability. Performance, of course, plays a very important rule in
>> Ignite,
>> > but it's possible to overdo anything.
>> >
>> > There are certainly parts of the project that can benefit from features
>> > like Stream API, without significant concern over performance. CLI is an
>> > obvious example, but I'm sure it's not the only one.
>> >
>> > That said, I don't think that banning something is productive. At the
>> same
>> > time, we should make sure we pay more attention to performance during
>> > reviews. Maybe we should have a checklist of major things to look for?
>> Not
>> > as a set of strict rules, but more as a guideline for contributors and
>> > committers.
>> >
>> > We might also want to start benchmarking the code and tracking the
>> progress.
>> >
>> > -Val
>> >
>> > On Wed, Sep 8, 2021 at 12:09 PM Pavel Tupitsyn 
>> wrote:
>> >
>> >> Alexander, Ivan,
>> >>
>> >>> not very productive to assume that 100% of your code is
>> >>> on the hot path
>> >>
>> >> That is exactly what we should be doing.
>> >> When I joined Ignite community 6 years ago, this was the prevalent
>> mindset.
>> >>
>> >> I'm not sure which part of Ignite can be considered "not on a hot
>> path".
>> >> Create/alter table (mentioned above) should perform well too.
>> >>
>> >>> measured first and only then optimized
>> >>> https://wiki.c2.com/?OptimizeLater
>> >>
>> >> Extra allocations are a "death by a thousand cuts".
>> >> They add up here and there, and then there are GC pauses.
>> >> This would be hard to "optimize later".
>> >>
>> >> It is common for perf-oriented projects to avoid some techniques.
>> >> For example, LINQ (streams analog from C# with similar perf issues) is
>> >> avoided in libraries and compilers [1].
>> >>
>> >> [1] https://github.com/dotnet/roslyn/blob/main/CONTRIBUTING.md
>> >>
>> >> On Wed, Sep 8, 2021 at 9:49 PM Valentin Kulichenko <
>> >> valentin.kuliche...@gmail.com> wrote:
>> >>
>> >>> I don't think we should ban anything. Streams API is just a tool in
>> the
>> >>> toolbox - it should be used appropriately. It's up to the contributor
>> and
>> >>> reviewer(s) to identify whether a particular usage might cause
>> >> performance
>> >>> issues.
>> >>>
>> >>> -Val
>> >>>
>> >>> On Wed, Sep 8, 2021 at 8:01 AM Alexander Polovtcev <
>> >>> alexpolovt...@gmail.com>
>> >>> wrote:
>> >>>
>>  -1
>>  I think that it is not very productive to assume that 100% of your
>> code
>> >>> is
>>  on the hot path, it would be impossible to write and maintain. Humans
>> >> are
>>  not very good at guessing where the performance bottlenecks are, so
>> the
>>  performance of the possible hot paths should be measured first and
>> only
>>  then optimized and documented.
>> 
>>  On Wed, Sep 8, 2021 at 5:53 PM Ivan Pavlukhin 
>> >>> wrote:
>> 
>> > Does not this trivial strategy work for us?
>> > https://wiki.c2.com/?OptimizeLater
>> >
>> > 2021-09-08 13:52 GMT+03:00, Andrey Gura :
>> >> Agree that any additional object creation on a hot path could be a
>> >> problem. So hot path should not contain stream API and any other
>> >> potentially problem code (e.g. iterator instead of for by index).
>> >>
>> >> On Wed, Sep 8, 2021 at 1:45 PM Pavel Tupitsyn <
>> >> ptupit...@apache.org>
>> > wrote:
>> >>>
>> >>> Ok, maybe a total ban is overkill, but right now streams are used
>> >>> even
>> > on
>> >>> some hot paths like getAllAsync [1].
>> >>>
>> >>> Another issue is that Collectors.toList() and other variants don't
>> > accept
>> >>> capacity, and we end up with unnecessary reallocations of
>> >> underlying
>> >>> arrays.
>> >>>
>> >>> [1]
>> >>>
>> >
>> 
>> >>>
>> >>
>> https://github.com/apache/ignite-3/blob/1d7d703ff2b18234b15a9a7b03104fbb73388edf/modules/table/src/main/java/org/apache/ignite/internal/table/KVBinaryViewImpl.java#L83
>> >>>
>> >>> On Wed, Sep 8, 2021 at 1:06 PM Konstantin Orlov <
>> >>> kor...@gridgain.com>
>> >>> wrote:
>>

Re: Ban Java Streams usage in Ignite 3 code

2021-09-09 Thread Ilya Kasnacheev
Hello!

-1 Let's not ban Java Streams, for the reasons already listed.

Regards,
-- 
Ilya Kasnacheev


чт, 9 сент. 2021 г. в 10:59, Ivan Daschinsky :

> Few words about the topic.
> There is a disease, quite common in the java community -- it is called the
> streamosis.
> But, to be honest, afear of streams is also not good.
>
> As for me, sometimes rewriting code completely with simple loops often
> makes it more readable, understandable and usually faster.
>
> So I am against a complete ban of streams, but I am for using this tool
> with caution. Often streams make code ugly and non-readable at all.
>
>
> чт, 9 сент. 2021 г. в 10:50, Ivan Daschinsky :
>
> > To be honest, Pavel, your benchmark is not quite correct. Please, rewrite
> > it using BlackHole
> >
> > чт, 9 сент. 2021 г. в 10:28, Nikolay Izhikov :
> >
> >> +1 to ban Streams usage.
> >>
> >>
> >>
> >> > 9 сент. 2021 г., в 02:59, Valentin Kulichenko <
> >> valentin.kuliche...@gmail.com> написал(а):
> >> >
> >> > Pavel,
> >> >
> >> > Quite frankly, I think we used to lean into performance too much. We
> >> > generally preferred it over data consistency, project modularity and
> >> code
> >> > readability. Performance, of course, plays a very important rule in
> >> Ignite,
> >> > but it's possible to overdo anything.
> >> >
> >> > There are certainly parts of the project that can benefit from
> features
> >> > like Stream API, without significant concern over performance. CLI is
> an
> >> > obvious example, but I'm sure it's not the only one.
> >> >
> >> > That said, I don't think that banning something is productive. At the
> >> same
> >> > time, we should make sure we pay more attention to performance during
> >> > reviews. Maybe we should have a checklist of major things to look for?
> >> Not
> >> > as a set of strict rules, but more as a guideline for contributors and
> >> > committers.
> >> >
> >> > We might also want to start benchmarking the code and tracking the
> >> progress.
> >> >
> >> > -Val
> >> >
> >> > On Wed, Sep 8, 2021 at 12:09 PM Pavel Tupitsyn 
> >> wrote:
> >> >
> >> >> Alexander, Ivan,
> >> >>
> >> >>> not very productive to assume that 100% of your code is
> >> >>> on the hot path
> >> >>
> >> >> That is exactly what we should be doing.
> >> >> When I joined Ignite community 6 years ago, this was the prevalent
> >> mindset.
> >> >>
> >> >> I'm not sure which part of Ignite can be considered "not on a hot
> >> path".
> >> >> Create/alter table (mentioned above) should perform well too.
> >> >>
> >> >>> measured first and only then optimized
> >> >>> https://wiki.c2.com/?OptimizeLater
> >> >>
> >> >> Extra allocations are a "death by a thousand cuts".
> >> >> They add up here and there, and then there are GC pauses.
> >> >> This would be hard to "optimize later".
> >> >>
> >> >> It is common for perf-oriented projects to avoid some techniques.
> >> >> For example, LINQ (streams analog from C# with similar perf issues)
> is
> >> >> avoided in libraries and compilers [1].
> >> >>
> >> >> [1] https://github.com/dotnet/roslyn/blob/main/CONTRIBUTING.md
> >> >>
> >> >> On Wed, Sep 8, 2021 at 9:49 PM Valentin Kulichenko <
> >> >> valentin.kuliche...@gmail.com> wrote:
> >> >>
> >> >>> I don't think we should ban anything. Streams API is just a tool in
> >> the
> >> >>> toolbox - it should be used appropriately. It's up to the
> contributor
> >> and
> >> >>> reviewer(s) to identify whether a particular usage might cause
> >> >> performance
> >> >>> issues.
> >> >>>
> >> >>> -Val
> >> >>>
> >> >>> On Wed, Sep 8, 2021 at 8:01 AM Alexander Polovtcev <
> >> >>> alexpolovt...@gmail.com>
> >> >>> wrote:
> >> >>>
> >>  -1
> >>  I think that it is not very productive to assume that 100% of your
> >> code
> >> >>> is
> >>  on the hot path, it would be impossible to write and maintain.
> Humans
> >> >> are
> >>  not very good at guessing where the performance bottlenecks are, so
> >> the
> >>  performance of the possible hot paths should be measured first and
> >> only
> >>  then optimized and documented.
> >> 
> >>  On Wed, Sep 8, 2021 at 5:53 PM Ivan Pavlukhin  >
> >> >>> wrote:
> >> 
> >> > Does not this trivial strategy work for us?
> >> > https://wiki.c2.com/?OptimizeLater
> >> >
> >> > 2021-09-08 13:52 GMT+03:00, Andrey Gura :
> >> >> Agree that any additional object creation on a hot path could be
> a
> >> >> problem. So hot path should not contain stream API and any other
> >> >> potentially problem code (e.g. iterator instead of for by index).
> >> >>
> >> >> On Wed, Sep 8, 2021 at 1:45 PM Pavel Tupitsyn <
> >> >> ptupit...@apache.org>
> >> > wrote:
> >> >>>
> >> >>> Ok, maybe a total ban is overkill, but right now streams are
> used
> >> >>> even
> >> > on
> >> >>> some hot paths like getAllAsync [1].
> >> >>>
> >> >>> Another issue is that Collectors.toList() and other variants
> don't
> >> > accept
> >> >>> capacity, and we end u

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Ivan Daschinsky
>> 2. In languages with proper async support (async-await, etc.), we can
skip sync API altogether.
It sounds pretty strange, as for me. Usually you cannot mix both functions
easily, it is called blue-red functions problem [1]
In python you definitely cannot do sync over async, for example
(principally can, but nobody do that because of mediocre performance of
that solution). And many developers
still prefer writing code withou asyncio at all.

Pavel, is it really true, that in .NET sync versions of libraries and tools
are completely eliminated?

[1] --
https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d

ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn :

> To put it another way:
> - true sync operation completes by itself
> - sync-over-async operation requires another thread to complete
>
> On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn 
> wrote:
>
> > Val,
> >
> > That's exactly what I have in mind.
> > Yes, we block the user thread, but then we should unblock it by
> completing
> > the future.
> > We can't complete the future from a Netty thread [1], so we'll need some
> > other thread from some executor.
> > If there are no threads available (because they are blocked by the sync
> > API above), the future won't complete => deadlock.
> >
> > [1]
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> >
> > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > valentin.kuliche...@gmail.com> wrote:
> >
> >> Pavel,
> >>
> >> I might be missing something - could you please elaborate a little more?
> >>
> >> When I say "sync on top of async", I basically mean that (for example)
> >> 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my
> >> understanding, it only blocks the user's thread.
> >>
> >> Am I wrong? Or you have a different implementation in mind?
> >>
> >> -Val
> >>
> >> On Wed, Sep 8, 2021 at 12:50 AM Pavel Tupitsyn 
> >> wrote:
> >>
> >> > Val,
> >> >
> >> > Agree with your points.
> >> >
> >> >
> >> > > async API should be primary
> >> >
> >> > It should be noted that all our APIs are inherently async,
> >> > because thin client is implemented asynchronously.
> >> >
> >> >
> >> > > with the sync version build on top
> >> >
> >> > We should document somehow that sync APIs are based on async ones,
> >> > because this may be dangerous in some use cases.
> >> >
> >> > For example, as a user, I may have a thread pool of 4 threads for
> >> > Ignite-related usage, that is also set as asyncContinuationExecutor
> [1].
> >> > Now if I run a lot of concurrent Ignite requests using sync API, all 4
> >> > threads will end up blocked on CompletableFutures.
> >> > When one of the operations completes, we enqueue the completion to
> that
> >> > same thread pool, but all threads are blocked on sync APIs, resulting
> >> in a
> >> > deadlock.
> >> >
> >> > This can be prevented by using a different asyncContinuationExecutor,
> >> but
> >> > sync API users won't be usually aware of this.
> >> >
> >> >
> >> > [1] https://issues.apache.org/jira/browse/IGNITE-15359
> >> >
> >> > On Wed, Sep 8, 2021 at 10:04 AM Courtney Robinson <
> >> > courtney.robin...@hypi.io>
> >> > wrote:
> >> >
> >> > > Hi Val,
> >> > >
> >> > > I'd highly support an async first API based on CompletionStage
> >> > > <
> >> > >
> >> >
> >>
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html
> >> > > >
> >> > > or
> >> > > its subtypes like CompletableFuture.
> >> > > In Ignite 2 we've written a wrapper library around IgniteFuture to
> >> > provide
> >> > > CompletionStage instead because many of the newer libs we use
> support
> >> > this.
> >> > > If Ignite 3 went this way it'd remove a lot of boiler plate/wrapper
> >> that
> >> > we
> >> > > wrote to get what you're suggesting here.
> >> > >
> >> > > Regards,
> >> > > Courtney Robinson
> >> > > Founder and CEO, Hypi
> >> > > Tel: ++44 208 123 2413 (GMT+0) 
> >> > >
> >> > > 
> >> > > https://hypi.io
> >> > >
> >> > >
> >> > > On Wed, Sep 8, 2021 at 12:44 AM Valentin Kulichenko <
> >> > > valentin.kuliche...@gmail.com> wrote:
> >> > >
> >> > > > Igniters,
> >> > > >
> >> > > > I would like to gather some opinions on whether we want to focus
> on
> >> > sync
> >> > > vs
> >> > > > async APIs in Ignite 3.
> >> > > >
> >> > > > Here are some initial considerations that I have:
> >> > > > 1. Ignite 2.x is essentially "sync first". Async APIs exist, but
> >> they
> >> > use
> >> > > > non-standard IgniteFuture and provide counterintuitive guarantees.
> >> In
> >> > my
> >> > > > experience, they significantly lack usability, and because of that
> >> are
> >> > > > rarely used.
> >> > > > 2. In general, however, async execution becomes more and more
> >> > prominent.
> >> > > > Something we can't ignore if we want to create a modern framework.
> >> > > > 3. Still, async support in Java is very limited (especially if
> >> compared
>

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Pavel Tupitsyn
Ivan,

> Pavel, is it really true, that in .NET sync versions of libraries and
tools
> are completely eliminated?

Far from being eliminated, but there is a movement in this direction.
For example, HttpClient [1] only provides async variants for most of the
methods.

Exposing sync wrappers for async methods is not recommended [2].
There is a good explanation of potential threading issues there, and it can
be applied to Java too.


> you cannot mix both functions easily

You can mix them easily.
C#: table.GetAsync(k).Result or table.GetAsync(k).GetAwaiter().GetResult()
(different exception handling)
Java: table.getAsync(k).get()

The same thing we do in sync wrapper for async method is now in the user
code.
Which is better for two reasons:
- users won't accidentally use sync API when async API should be used
- when they really have to use sync API, potentially dangerous behavior is
not hidden


[1]
https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
[2]
https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/

On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky  wrote:

> >> 2. In languages with proper async support (async-await, etc.), we can
> skip sync API altogether.
> It sounds pretty strange, as for me. Usually you cannot mix both functions
> easily, it is called blue-red functions problem [1]
> In python you definitely cannot do sync over async, for example
> (principally can, but nobody do that because of mediocre performance of
> that solution). And many developers
> still prefer writing code withou asyncio at all.
>
> Pavel, is it really true, that in .NET sync versions of libraries and tools
> are completely eliminated?
>
> [1] --
> https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
>
> ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn :
>
> > To put it another way:
> > - true sync operation completes by itself
> > - sync-over-async operation requires another thread to complete
> >
> > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn 
> > wrote:
> >
> > > Val,
> > >
> > > That's exactly what I have in mind.
> > > Yes, we block the user thread, but then we should unblock it by
> > completing
> > > the future.
> > > We can't complete the future from a Netty thread [1], so we'll need
> some
> > > other thread from some executor.
> > > If there are no threads available (because they are blocked by the sync
> > > API above), the future won't complete => deadlock.
> > >
> > > [1]
> > >
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> > >
> > > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > > valentin.kuliche...@gmail.com> wrote:
> > >
> > >> Pavel,
> > >>
> > >> I might be missing something - could you please elaborate a little
> more?
> > >>
> > >> When I say "sync on top of async", I basically mean that (for example)
> > >> 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my
> > >> understanding, it only blocks the user's thread.
> > >>
> > >> Am I wrong? Or you have a different implementation in mind?
> > >>
> > >> -Val
> > >>
> > >> On Wed, Sep 8, 2021 at 12:50 AM Pavel Tupitsyn 
> > >> wrote:
> > >>
> > >> > Val,
> > >> >
> > >> > Agree with your points.
> > >> >
> > >> >
> > >> > > async API should be primary
> > >> >
> > >> > It should be noted that all our APIs are inherently async,
> > >> > because thin client is implemented asynchronously.
> > >> >
> > >> >
> > >> > > with the sync version build on top
> > >> >
> > >> > We should document somehow that sync APIs are based on async ones,
> > >> > because this may be dangerous in some use cases.
> > >> >
> > >> > For example, as a user, I may have a thread pool of 4 threads for
> > >> > Ignite-related usage, that is also set as asyncContinuationExecutor
> > [1].
> > >> > Now if I run a lot of concurrent Ignite requests using sync API,
> all 4
> > >> > threads will end up blocked on CompletableFutures.
> > >> > When one of the operations completes, we enqueue the completion to
> > that
> > >> > same thread pool, but all threads are blocked on sync APIs,
> resulting
> > >> in a
> > >> > deadlock.
> > >> >
> > >> > This can be prevented by using a different
> asyncContinuationExecutor,
> > >> but
> > >> > sync API users won't be usually aware of this.
> > >> >
> > >> >
> > >> > [1] https://issues.apache.org/jira/browse/IGNITE-15359
> > >> >
> > >> > On Wed, Sep 8, 2021 at 10:04 AM Courtney Robinson <
> > >> > courtney.robin...@hypi.io>
> > >> > wrote:
> > >> >
> > >> > > Hi Val,
> > >> > >
> > >> > > I'd highly support an async first API based on CompletionStage
> > >> > > <
> > >> > >
> > >> >
> > >>
> >
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletionStage.html
> > >> > > >
> > >> > > or
> > >> > > its subtypes like CompletableFuture.
> > >> > > In Ignite 2 we've written a wrapper library around IgniteFuture to
> > >> > prov

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Ivan Daschinsky
>> You can mix them easily.
This is far from easily (you have already mentioned continuation problem),
but for i.e. in python it is absolutely not.
For kotlin it is a little bit easier, but also not fluent and a little bit
ugly.

чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn :

> Ivan,
>
> > Pavel, is it really true, that in .NET sync versions of libraries and
> tools
> > are completely eliminated?
>
> Far from being eliminated, but there is a movement in this direction.
> For example, HttpClient [1] only provides async variants for most of the
> methods.
>
> Exposing sync wrappers for async methods is not recommended [2].
> There is a good explanation of potential threading issues there, and it can
> be applied to Java too.
>
>
> > you cannot mix both functions easily
>
> You can mix them easily.
> C#: table.GetAsync(k).Result or table.GetAsync(k).GetAwaiter().GetResult()
> (different exception handling)
> Java: table.getAsync(k).get()
>
> The same thing we do in sync wrapper for async method is now in the user
> code.
> Which is better for two reasons:
> - users won't accidentally use sync API when async API should be used
> - when they really have to use sync API, potentially dangerous behavior is
> not hidden
>
>
> [1]
>
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> [2]
>
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
>
> On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky 
> wrote:
>
> > >> 2. In languages with proper async support (async-await, etc.), we can
> > skip sync API altogether.
> > It sounds pretty strange, as for me. Usually you cannot mix both
> functions
> > easily, it is called blue-red functions problem [1]
> > In python you definitely cannot do sync over async, for example
> > (principally can, but nobody do that because of mediocre performance of
> > that solution). And many developers
> > still prefer writing code withou asyncio at all.
> >
> > Pavel, is it really true, that in .NET sync versions of libraries and
> tools
> > are completely eliminated?
> >
> > [1] --
> > https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> >
> > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn :
> >
> > > To put it another way:
> > > - true sync operation completes by itself
> > > - sync-over-async operation requires another thread to complete
> > >
> > > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn 
> > > wrote:
> > >
> > > > Val,
> > > >
> > > > That's exactly what I have in mind.
> > > > Yes, we block the user thread, but then we should unblock it by
> > > completing
> > > > the future.
> > > > We can't complete the future from a Netty thread [1], so we'll need
> > some
> > > > other thread from some executor.
> > > > If there are no threads available (because they are blocked by the
> sync
> > > > API above), the future won't complete => deadlock.
> > > >
> > > > [1]
> > > >
> > >
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> > > >
> > > > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > > > valentin.kuliche...@gmail.com> wrote:
> > > >
> > > >> Pavel,
> > > >>
> > > >> I might be missing something - could you please elaborate a little
> > more?
> > > >>
> > > >> When I say "sync on top of async", I basically mean that (for
> example)
> > > >> 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my
> > > >> understanding, it only blocks the user's thread.
> > > >>
> > > >> Am I wrong? Or you have a different implementation in mind?
> > > >>
> > > >> -Val
> > > >>
> > > >> On Wed, Sep 8, 2021 at 12:50 AM Pavel Tupitsyn <
> ptupit...@apache.org>
> > > >> wrote:
> > > >>
> > > >> > Val,
> > > >> >
> > > >> > Agree with your points.
> > > >> >
> > > >> >
> > > >> > > async API should be primary
> > > >> >
> > > >> > It should be noted that all our APIs are inherently async,
> > > >> > because thin client is implemented asynchronously.
> > > >> >
> > > >> >
> > > >> > > with the sync version build on top
> > > >> >
> > > >> > We should document somehow that sync APIs are based on async ones,
> > > >> > because this may be dangerous in some use cases.
> > > >> >
> > > >> > For example, as a user, I may have a thread pool of 4 threads for
> > > >> > Ignite-related usage, that is also set as
> asyncContinuationExecutor
> > > [1].
> > > >> > Now if I run a lot of concurrent Ignite requests using sync API,
> > all 4
> > > >> > threads will end up blocked on CompletableFutures.
> > > >> > When one of the operations completes, we enqueue the completion to
> > > that
> > > >> > same thread pool, but all threads are blocked on sync APIs,
> > resulting
> > > >> in a
> > > >> > deadlock.
> > > >> >
> > > >> > This can be prevented by using a different
> > asyncContinuationExecutor,
> > > >> but
> > > >> > sync API users won't be usually aware of this.
> > > >> >
> > > >> >
> > > >> > [1] https:/

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Pavel Tupitsyn
Talked to Ivan in private, and came up with the following per-language
summary:

* Java: sync and async
*. NET: only async
* Python: sync and async
* JavaScript: only async (sync is not possible)

Thin clients in other languages are to be discussed.

On Thu, Sep 9, 2021 at 11:49 AM Ivan Daschinsky  wrote:

> >> You can mix them easily.
> This is far from easily (you have already mentioned continuation problem),
> but for i.e. in python it is absolutely not.
> For kotlin it is a little bit easier, but also not fluent and a little bit
> ugly.
>
> чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn :
>
> > Ivan,
> >
> > > Pavel, is it really true, that in .NET sync versions of libraries and
> > tools
> > > are completely eliminated?
> >
> > Far from being eliminated, but there is a movement in this direction.
> > For example, HttpClient [1] only provides async variants for most of the
> > methods.
> >
> > Exposing sync wrappers for async methods is not recommended [2].
> > There is a good explanation of potential threading issues there, and it
> can
> > be applied to Java too.
> >
> >
> > > you cannot mix both functions easily
> >
> > You can mix them easily.
> > C#: table.GetAsync(k).Result or
> table.GetAsync(k).GetAwaiter().GetResult()
> > (different exception handling)
> > Java: table.getAsync(k).get()
> >
> > The same thing we do in sync wrapper for async method is now in the user
> > code.
> > Which is better for two reasons:
> > - users won't accidentally use sync API when async API should be used
> > - when they really have to use sync API, potentially dangerous behavior
> is
> > not hidden
> >
> >
> > [1]
> >
> >
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> > [2]
> >
> >
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
> >
> > On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky 
> > wrote:
> >
> > > >> 2. In languages with proper async support (async-await, etc.), we
> can
> > > skip sync API altogether.
> > > It sounds pretty strange, as for me. Usually you cannot mix both
> > functions
> > > easily, it is called blue-red functions problem [1]
> > > In python you definitely cannot do sync over async, for example
> > > (principally can, but nobody do that because of mediocre performance of
> > > that solution). And many developers
> > > still prefer writing code withou asyncio at all.
> > >
> > > Pavel, is it really true, that in .NET sync versions of libraries and
> > tools
> > > are completely eliminated?
> > >
> > > [1] --
> > >
> https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> > >
> > > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn :
> > >
> > > > To put it another way:
> > > > - true sync operation completes by itself
> > > > - sync-over-async operation requires another thread to complete
> > > >
> > > > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn  >
> > > > wrote:
> > > >
> > > > > Val,
> > > > >
> > > > > That's exactly what I have in mind.
> > > > > Yes, we block the user thread, but then we should unblock it by
> > > > completing
> > > > > the future.
> > > > > We can't complete the future from a Netty thread [1], so we'll need
> > > some
> > > > > other thread from some executor.
> > > > > If there are no threads available (because they are blocked by the
> > sync
> > > > > API above), the future won't complete => deadlock.
> > > > >
> > > > > [1]
> > > > >
> > > >
> > >
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> > > > >
> > > > > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > > > > valentin.kuliche...@gmail.com> wrote:
> > > > >
> > > > >> Pavel,
> > > > >>
> > > > >> I might be missing something - could you please elaborate a little
> > > more?
> > > > >>
> > > > >> When I say "sync on top of async", I basically mean that (for
> > example)
> > > > >> 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my
> > > > >> understanding, it only blocks the user's thread.
> > > > >>
> > > > >> Am I wrong? Or you have a different implementation in mind?
> > > > >>
> > > > >> -Val
> > > > >>
> > > > >> On Wed, Sep 8, 2021 at 12:50 AM Pavel Tupitsyn <
> > ptupit...@apache.org>
> > > > >> wrote:
> > > > >>
> > > > >> > Val,
> > > > >> >
> > > > >> > Agree with your points.
> > > > >> >
> > > > >> >
> > > > >> > > async API should be primary
> > > > >> >
> > > > >> > It should be noted that all our APIs are inherently async,
> > > > >> > because thin client is implemented asynchronously.
> > > > >> >
> > > > >> >
> > > > >> > > with the sync version build on top
> > > > >> >
> > > > >> > We should document somehow that sync APIs are based on async
> ones,
> > > > >> > because this may be dangerous in some use cases.
> > > > >> >
> > > > >> > For example, as a user, I may have a thread pool of 4 threads
> for
> > > > >> > Ignite-related usage, that is also set as
> > asyncContin

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Ivan Daschinsky
And what about C/C++?  There are so many options for them...

чт, 9 сент. 2021 г. в 13:00, Pavel Tupitsyn :

> Talked to Ivan in private, and came up with the following per-language
> summary:
>
> * Java: sync and async
> *. NET: only async
> * Python: sync and async
> * JavaScript: only async (sync is not possible)
>
> Thin clients in other languages are to be discussed.
>
> On Thu, Sep 9, 2021 at 11:49 AM Ivan Daschinsky 
> wrote:
>
> > >> You can mix them easily.
> > This is far from easily (you have already mentioned continuation
> problem),
> > but for i.e. in python it is absolutely not.
> > For kotlin it is a little bit easier, but also not fluent and a little
> bit
> > ugly.
> >
> > чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn :
> >
> > > Ivan,
> > >
> > > > Pavel, is it really true, that in .NET sync versions of libraries and
> > > tools
> > > > are completely eliminated?
> > >
> > > Far from being eliminated, but there is a movement in this direction.
> > > For example, HttpClient [1] only provides async variants for most of
> the
> > > methods.
> > >
> > > Exposing sync wrappers for async methods is not recommended [2].
> > > There is a good explanation of potential threading issues there, and it
> > can
> > > be applied to Java too.
> > >
> > >
> > > > you cannot mix both functions easily
> > >
> > > You can mix them easily.
> > > C#: table.GetAsync(k).Result or
> > table.GetAsync(k).GetAwaiter().GetResult()
> > > (different exception handling)
> > > Java: table.getAsync(k).get()
> > >
> > > The same thing we do in sync wrapper for async method is now in the
> user
> > > code.
> > > Which is better for two reasons:
> > > - users won't accidentally use sync API when async API should be used
> > > - when they really have to use sync API, potentially dangerous behavior
> > is
> > > not hidden
> > >
> > >
> > > [1]
> > >
> > >
> >
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> > > [2]
> > >
> > >
> >
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
> > >
> > > On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky 
> > > wrote:
> > >
> > > > >> 2. In languages with proper async support (async-await, etc.), we
> > can
> > > > skip sync API altogether.
> > > > It sounds pretty strange, as for me. Usually you cannot mix both
> > > functions
> > > > easily, it is called blue-red functions problem [1]
> > > > In python you definitely cannot do sync over async, for example
> > > > (principally can, but nobody do that because of mediocre performance
> of
> > > > that solution). And many developers
> > > > still prefer writing code withou asyncio at all.
> > > >
> > > > Pavel, is it really true, that in .NET sync versions of libraries and
> > > tools
> > > > are completely eliminated?
> > > >
> > > > [1] --
> > > >
> > https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> > > >
> > > > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn :
> > > >
> > > > > To put it another way:
> > > > > - true sync operation completes by itself
> > > > > - sync-over-async operation requires another thread to complete
> > > > >
> > > > > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn <
> ptupit...@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > > > Val,
> > > > > >
> > > > > > That's exactly what I have in mind.
> > > > > > Yes, we block the user thread, but then we should unblock it by
> > > > > completing
> > > > > > the future.
> > > > > > We can't complete the future from a Netty thread [1], so we'll
> need
> > > > some
> > > > > > other thread from some executor.
> > > > > > If there are no threads available (because they are blocked by
> the
> > > sync
> > > > > > API above), the future won't complete => deadlock.
> > > > > >
> > > > > > [1]
> > > > > >
> > > > >
> > > >
> > >
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> > > > > >
> > > > > > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > > > > > valentin.kuliche...@gmail.com> wrote:
> > > > > >
> > > > > >> Pavel,
> > > > > >>
> > > > > >> I might be missing something - could you please elaborate a
> little
> > > > more?
> > > > > >>
> > > > > >> When I say "sync on top of async", I basically mean that (for
> > > example)
> > > > > >> 'insert(..)' is equivalent to 'insertAsync(..).join()'. In my
> > > > > >> understanding, it only blocks the user's thread.
> > > > > >>
> > > > > >> Am I wrong? Or you have a different implementation in mind?
> > > > > >>
> > > > > >> -Val
> > > > > >>
> > > > > >> On Wed, Sep 8, 2021 at 12:50 AM Pavel Tupitsyn <
> > > ptupit...@apache.org>
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Val,
> > > > > >> >
> > > > > >> > Agree with your points.
> > > > > >> >
> > > > > >> >
> > > > > >> > > async API should be primary
> > > > > >> >
> > > > > >> > It should be noted that all our APIs are inherently async,
> > > > > >> > becau

Release of pyignite 0.5.2 proposal

2021-09-09 Thread Ivan Daschinsky
Hi, folks.

Unfortunately, a quite severe bug found in pyignite since 0.4.0 was found
[1]. Fortunately, it is already fixed. Also there are also a few bugs
already fixed.

I propose to prepare the next minor release. Branch was already cut and
commits were cherry-picked [3].

If there are no objections, I will prepare a release and will start voting
thread in a day or two.

[1] -- https://issues.apache.org/jira/browse/IGNITE-15479
[2] --
https://issues.apache.org/jira/issues/?jql=project%20%3D%20IGNITE%20AND%20fixVersion%20%3D%20python-0.5.2
[3] --
https://github.com/apache/ignite-python-thin-client/commits/pyignite-0.5.2


Re: Release of pyignite 0.5.2 proposal

2021-09-09 Thread Ivan Daschinsky
TC build of release branch --
https://tc.sbt-ignite-dev.ru/buildConfiguration/IgniteThinClients_Tests_ThinClientPython/6130289

чт, 9 сент. 2021 г. в 13:40, Ivan Daschinsky :

> Hi, folks.
>
> Unfortunately, a quite severe bug found in pyignite since 0.4.0 was found
> [1]. Fortunately, it is already fixed. Also there are also a few bugs
> already fixed.
>
> I propose to prepare the next minor release. Branch was already cut and
> commits were cherry-picked [3].
>
> If there are no objections, I will prepare a release and will start voting
> thread in a day or two.
>
> [1] -- https://issues.apache.org/jira/browse/IGNITE-15479
> [2] --
> https://issues.apache.org/jira/issues/?jql=project%20%3D%20IGNITE%20AND%20fixVersion%20%3D%20python-0.5.2
> [3] --
> https://github.com/apache/ignite-python-thin-client/commits/pyignite-0.5.2
>


-- 
Sincerely yours, Ivan Daschinskiy


Re: Release of pyignite 0.5.2 proposal

2021-09-09 Thread Nikolay Izhikov
+1 to release ASAP.

> 9 сент. 2021 г., в 13:43, Ivan Daschinsky  написал(а):
> 
> TC build of release branch --
> https://tc.sbt-ignite-dev.ru/buildConfiguration/IgniteThinClients_Tests_ThinClientPython/6130289
> 
> чт, 9 сент. 2021 г. в 13:40, Ivan Daschinsky :
> 
>> Hi, folks.
>> 
>> Unfortunately, a quite severe bug found in pyignite since 0.4.0 was found
>> [1]. Fortunately, it is already fixed. Also there are also a few bugs
>> already fixed.
>> 
>> I propose to prepare the next minor release. Branch was already cut and
>> commits were cherry-picked [3].
>> 
>> If there are no objections, I will prepare a release and will start voting
>> thread in a day or two.
>> 
>> [1] -- https://issues.apache.org/jira/browse/IGNITE-15479
>> [2] --
>> https://issues.apache.org/jira/issues/?jql=project%20%3D%20IGNITE%20AND%20fixVersion%20%3D%20python-0.5.2
>> [3] --
>> https://github.com/apache/ignite-python-thin-client/commits/pyignite-0.5.2
>> 
> 
> 
> -- 
> Sincerely yours, Ivan Daschinskiy



Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Igor Sapego
Well, fortunately we do not provide a C client if you don't consider ODBC
as one so we should not think about it. For C++ I believe we should use
standard std::future+std::promise for async, but still can provide sync
methods,
built on top of async methods. There is no continuation problem in C++ API
as
std::future do not provide continuation methods :)

Now, for the sync/async problem you've mentioned. Can not we solve it by
using different thread pools for completion and continuation of futures or
does
CompletableFuture interface not allowing for that?

Best Regards,
Igor


On Thu, Sep 9, 2021 at 1:15 PM Ivan Daschinsky  wrote:

> And what about C/C++?  There are so many options for them...
>
> чт, 9 сент. 2021 г. в 13:00, Pavel Tupitsyn :
>
> > Talked to Ivan in private, and came up with the following per-language
> > summary:
> >
> > * Java: sync and async
> > *. NET: only async
> > * Python: sync and async
> > * JavaScript: only async (sync is not possible)
> >
> > Thin clients in other languages are to be discussed.
> >
> > On Thu, Sep 9, 2021 at 11:49 AM Ivan Daschinsky 
> > wrote:
> >
> > > >> You can mix them easily.
> > > This is far from easily (you have already mentioned continuation
> > problem),
> > > but for i.e. in python it is absolutely not.
> > > For kotlin it is a little bit easier, but also not fluent and a little
> > bit
> > > ugly.
> > >
> > > чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn :
> > >
> > > > Ivan,
> > > >
> > > > > Pavel, is it really true, that in .NET sync versions of libraries
> and
> > > > tools
> > > > > are completely eliminated?
> > > >
> > > > Far from being eliminated, but there is a movement in this direction.
> > > > For example, HttpClient [1] only provides async variants for most of
> > the
> > > > methods.
> > > >
> > > > Exposing sync wrappers for async methods is not recommended [2].
> > > > There is a good explanation of potential threading issues there, and
> it
> > > can
> > > > be applied to Java too.
> > > >
> > > >
> > > > > you cannot mix both functions easily
> > > >
> > > > You can mix them easily.
> > > > C#: table.GetAsync(k).Result or
> > > table.GetAsync(k).GetAwaiter().GetResult()
> > > > (different exception handling)
> > > > Java: table.getAsync(k).get()
> > > >
> > > > The same thing we do in sync wrapper for async method is now in the
> > user
> > > > code.
> > > > Which is better for two reasons:
> > > > - users won't accidentally use sync API when async API should be used
> > > > - when they really have to use sync API, potentially dangerous
> behavior
> > > is
> > > > not hidden
> > > >
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> > > > [2]
> > > >
> > > >
> > >
> >
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
> > > >
> > > > On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky  >
> > > > wrote:
> > > >
> > > > > >> 2. In languages with proper async support (async-await, etc.),
> we
> > > can
> > > > > skip sync API altogether.
> > > > > It sounds pretty strange, as for me. Usually you cannot mix both
> > > > functions
> > > > > easily, it is called blue-red functions problem [1]
> > > > > In python you definitely cannot do sync over async, for example
> > > > > (principally can, but nobody do that because of mediocre
> performance
> > of
> > > > > that solution). And many developers
> > > > > still prefer writing code withou asyncio at all.
> > > > >
> > > > > Pavel, is it really true, that in .NET sync versions of libraries
> and
> > > > tools
> > > > > are completely eliminated?
> > > > >
> > > > > [1] --
> > > > >
> > >
> https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> > > > >
> > > > > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn  >:
> > > > >
> > > > > > To put it another way:
> > > > > > - true sync operation completes by itself
> > > > > > - sync-over-async operation requires another thread to complete
> > > > > >
> > > > > > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn <
> > ptupit...@apache.org
> > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Val,
> > > > > > >
> > > > > > > That's exactly what I have in mind.
> > > > > > > Yes, we block the user thread, but then we should unblock it by
> > > > > > completing
> > > > > > > the future.
> > > > > > > We can't complete the future from a Netty thread [1], so we'll
> > need
> > > > > some
> > > > > > > other thread from some executor.
> > > > > > > If there are no threads available (because they are blocked by
> > the
> > > > sync
> > > > > > > API above), the future won't complete => deadlock.
> > > > > > >
> > > > > > > [1]
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://lists.apache.org/thread.html/r528659381d983a177d779f56ef3d7da6fe17eb3504383f5f87727514%40%3Cdev.ignite.apache.org%3E
> > > > > > >
> > > > > > > On Wed, Sep 8, 2021 at 9:40 PM Valentin Kulichenko <
> > > > > > > valen

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Ivan Daschinsky
Igor, and what about C++20 and coroutines [1]

[1] -- https://en.cppreference.com/w/cpp/language/coroutines

чт, 9 сент. 2021 г. в 14:12, Igor Sapego :

> Well, fortunately we do not provide a C client if you don't consider ODBC
> as one so we should not think about it. For C++ I believe we should use
> standard std::future+std::promise for async, but still can provide sync
> methods,
> built on top of async methods. There is no continuation problem in C++ API
> as
> std::future do not provide continuation methods :)
>
> Now, for the sync/async problem you've mentioned. Can not we solve it by
> using different thread pools for completion and continuation of futures or
> does
> CompletableFuture interface not allowing for that?
>
> Best Regards,
> Igor
>
>
> On Thu, Sep 9, 2021 at 1:15 PM Ivan Daschinsky 
> wrote:
>
> > And what about C/C++?  There are so many options for them...
> >
> > чт, 9 сент. 2021 г. в 13:00, Pavel Tupitsyn :
> >
> > > Talked to Ivan in private, and came up with the following per-language
> > > summary:
> > >
> > > * Java: sync and async
> > > *. NET: only async
> > > * Python: sync and async
> > > * JavaScript: only async (sync is not possible)
> > >
> > > Thin clients in other languages are to be discussed.
> > >
> > > On Thu, Sep 9, 2021 at 11:49 AM Ivan Daschinsky 
> > > wrote:
> > >
> > > > >> You can mix them easily.
> > > > This is far from easily (you have already mentioned continuation
> > > problem),
> > > > but for i.e. in python it is absolutely not.
> > > > For kotlin it is a little bit easier, but also not fluent and a
> little
> > > bit
> > > > ugly.
> > > >
> > > > чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn :
> > > >
> > > > > Ivan,
> > > > >
> > > > > > Pavel, is it really true, that in .NET sync versions of libraries
> > and
> > > > > tools
> > > > > > are completely eliminated?
> > > > >
> > > > > Far from being eliminated, but there is a movement in this
> direction.
> > > > > For example, HttpClient [1] only provides async variants for most
> of
> > > the
> > > > > methods.
> > > > >
> > > > > Exposing sync wrappers for async methods is not recommended [2].
> > > > > There is a good explanation of potential threading issues there,
> and
> > it
> > > > can
> > > > > be applied to Java too.
> > > > >
> > > > >
> > > > > > you cannot mix both functions easily
> > > > >
> > > > > You can mix them easily.
> > > > > C#: table.GetAsync(k).Result or
> > > > table.GetAsync(k).GetAwaiter().GetResult()
> > > > > (different exception handling)
> > > > > Java: table.getAsync(k).get()
> > > > >
> > > > > The same thing we do in sync wrapper for async method is now in the
> > > user
> > > > > code.
> > > > > Which is better for two reasons:
> > > > > - users won't accidentally use sync API when async API should be
> used
> > > > > - when they really have to use sync API, potentially dangerous
> > behavior
> > > > is
> > > > > not hidden
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > > >
> > > >
> > >
> >
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> > > > > [2]
> > > > >
> > > > >
> > > >
> > >
> >
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
> > > > >
> > > > > On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky <
> ivanda...@gmail.com
> > >
> > > > > wrote:
> > > > >
> > > > > > >> 2. In languages with proper async support (async-await, etc.),
> > we
> > > > can
> > > > > > skip sync API altogether.
> > > > > > It sounds pretty strange, as for me. Usually you cannot mix both
> > > > > functions
> > > > > > easily, it is called blue-red functions problem [1]
> > > > > > In python you definitely cannot do sync over async, for example
> > > > > > (principally can, but nobody do that because of mediocre
> > performance
> > > of
> > > > > > that solution). And many developers
> > > > > > still prefer writing code withou asyncio at all.
> > > > > >
> > > > > > Pavel, is it really true, that in .NET sync versions of libraries
> > and
> > > > > tools
> > > > > > are completely eliminated?
> > > > > >
> > > > > > [1] --
> > > > > >
> > > >
> > https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> > > > > >
> > > > > > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn <
> ptupit...@apache.org
> > >:
> > > > > >
> > > > > > > To put it another way:
> > > > > > > - true sync operation completes by itself
> > > > > > > - sync-over-async operation requires another thread to complete
> > > > > > >
> > > > > > > On Wed, Sep 8, 2021 at 10:15 PM Pavel Tupitsyn <
> > > ptupit...@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Val,
> > > > > > > >
> > > > > > > > That's exactly what I have in mind.
> > > > > > > > Yes, we block the user thread, but then we should unblock it
> by
> > > > > > > completing
> > > > > > > > the future.
> > > > > > > > We can't complete the future from a Netty thread [1], so
> we'll
> > > need
> > > > > > some
> > > > > > 

Re: Sync vs async APIs in Ignite 3

2021-09-09 Thread Igor Sapego
Ivan,

We are not going to adopt C++20 in the near future, as new standards
are not adopted as fast in C++ world and if we'd do that most probably
almost none of our users could use our client.

But yeah, if we'd use coroutines, I'd probably suggest that we only
provide the async API.

Best Regards,
Igor



On Thu, Sep 9, 2021 at 2:29 PM Ivan Daschinsky  wrote:

> Igor, and what about C++20 and coroutines [1]
>
> [1] -- https://en.cppreference.com/w/cpp/language/coroutines
>
> чт, 9 сент. 2021 г. в 14:12, Igor Sapego :
>
> > Well, fortunately we do not provide a C client if you don't consider ODBC
> > as one so we should not think about it. For C++ I believe we should use
> > standard std::future+std::promise for async, but still can provide sync
> > methods,
> > built on top of async methods. There is no continuation problem in C++
> API
> > as
> > std::future do not provide continuation methods :)
> >
> > Now, for the sync/async problem you've mentioned. Can not we solve it by
> > using different thread pools for completion and continuation of futures
> or
> > does
> > CompletableFuture interface not allowing for that?
> >
> > Best Regards,
> > Igor
> >
> >
> > On Thu, Sep 9, 2021 at 1:15 PM Ivan Daschinsky 
> > wrote:
> >
> > > And what about C/C++?  There are so many options for them...
> > >
> > > чт, 9 сент. 2021 г. в 13:00, Pavel Tupitsyn :
> > >
> > > > Talked to Ivan in private, and came up with the following
> per-language
> > > > summary:
> > > >
> > > > * Java: sync and async
> > > > *. NET: only async
> > > > * Python: sync and async
> > > > * JavaScript: only async (sync is not possible)
> > > >
> > > > Thin clients in other languages are to be discussed.
> > > >
> > > > On Thu, Sep 9, 2021 at 11:49 AM Ivan Daschinsky  >
> > > > wrote:
> > > >
> > > > > >> You can mix them easily.
> > > > > This is far from easily (you have already mentioned continuation
> > > > problem),
> > > > > but for i.e. in python it is absolutely not.
> > > > > For kotlin it is a little bit easier, but also not fluent and a
> > little
> > > > bit
> > > > > ugly.
> > > > >
> > > > > чт, 9 сент. 2021 г. в 11:37, Pavel Tupitsyn  >:
> > > > >
> > > > > > Ivan,
> > > > > >
> > > > > > > Pavel, is it really true, that in .NET sync versions of
> libraries
> > > and
> > > > > > tools
> > > > > > > are completely eliminated?
> > > > > >
> > > > > > Far from being eliminated, but there is a movement in this
> > direction.
> > > > > > For example, HttpClient [1] only provides async variants for most
> > of
> > > > the
> > > > > > methods.
> > > > > >
> > > > > > Exposing sync wrappers for async methods is not recommended [2].
> > > > > > There is a good explanation of potential threading issues there,
> > and
> > > it
> > > > > can
> > > > > > be applied to Java too.
> > > > > >
> > > > > >
> > > > > > > you cannot mix both functions easily
> > > > > >
> > > > > > You can mix them easily.
> > > > > > C#: table.GetAsync(k).Result or
> > > > > table.GetAsync(k).GetAwaiter().GetResult()
> > > > > > (different exception handling)
> > > > > > Java: table.getAsync(k).get()
> > > > > >
> > > > > > The same thing we do in sync wrapper for async method is now in
> the
> > > > user
> > > > > > code.
> > > > > > Which is better for two reasons:
> > > > > > - users won't accidentally use sync API when async API should be
> > used
> > > > > > - when they really have to use sync API, potentially dangerous
> > > behavior
> > > > > is
> > > > > > not hidden
> > > > > >
> > > > > >
> > > > > > [1]
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-5.0#methods
> > > > > > [2]
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/
> > > > > >
> > > > > > On Thu, Sep 9, 2021 at 11:16 AM Ivan Daschinsky <
> > ivanda...@gmail.com
> > > >
> > > > > > wrote:
> > > > > >
> > > > > > > >> 2. In languages with proper async support (async-await,
> etc.),
> > > we
> > > > > can
> > > > > > > skip sync API altogether.
> > > > > > > It sounds pretty strange, as for me. Usually you cannot mix
> both
> > > > > > functions
> > > > > > > easily, it is called blue-red functions problem [1]
> > > > > > > In python you definitely cannot do sync over async, for example
> > > > > > > (principally can, but nobody do that because of mediocre
> > > performance
> > > > of
> > > > > > > that solution). And many developers
> > > > > > > still prefer writing code withou asyncio at all.
> > > > > > >
> > > > > > > Pavel, is it really true, that in .NET sync versions of
> libraries
> > > and
> > > > > > tools
> > > > > > > are completely eliminated?
> > > > > > >
> > > > > > > [1] --
> > > > > > >
> > > > >
> > >
> https://elizarov.medium.com/how-do-you-color-your-functions-a6bb423d936d
> > > > > > >
> > > > > > > ср, 8 сент. 2021 г. в 22:33, Pavel Tupitsyn <
> > ptupit...@apache.org
> > > >:
> >

Replace Map with List and Iterable in KeyValueView Ignite 3 APIs

2021-09-09 Thread Pavel Tupitsyn
Igniters,

I propose to replace Map with List in getAll and invokeAll, and
Iterable in putAll APIs of Ignite 3.x KeyValueView.

1. Performance
putAll simply iterates over the map, we can easily accept Iterable instead.
Iterable can be implemented over anything, it can lazily read data from a
file or some other place, instead of allocating a huge collection and
performing unnecessary hashing.

getAll returns a Map, but we don't know if the user code needs a map or
just wants to iterate over the results, in which case Map is just overhead.

2. Equality
getAll returns Map, but in many cases, the map will be useless
because K does not have proper equals()/hashCode() implementation, so
map.get(key) does not work.

Notes:
- It is not clear which Pair class to use yet - IgniteBiTuple or something
else.
- Ignite 3 won't deadlock due to putAll entry order, so we don't have to
worry about sorting.

Thoughts, objections?


Tuple equality in Ignite 3.x

2021-09-09 Thread Pavel Tupitsyn
Igniters,

Tuple in Ignite 3.x is a replacement for BinaryObject in Ignite 2.x.
Let's discuss equality and sorting.

- We have multiple Tuple implementations, and our API allows custom,
user-defined Tuples as well (which can be useful for performance when
bridging Ignite with another system or importing the data from somewhere).
- We don't have equals()/hashCode() overrides, so it is not possible to
compare tuples or put them into a Map.

Proposal:
- Add public TupleComparator implements Comparator, based on the
tuple contents (column names and values)
- Implement common TupleComparator#hashCode(Tuple t) method that combines
hash codes of column names and values
- Implement equals(), hashCode(), and Comparable on all built-in tuples,
delegate the logic to TupleComparator
- Make Tuple extend Compable

This way we cover all sorting/comparing/mapping scenarios for built-in
tuples and provide reusable code for third-party implementations.

Thoughts?


Re: Replace Map with List and Iterable in KeyValueView Ignite 3 APIs

2021-09-09 Thread Alexei Scherbakov
Pavel,

I think the current API looks more natural compared to your proposal.

-1  from my side, see comments below.

чт, 9 сент. 2021 г. в 15:38, Pavel Tupitsyn :

> Igniters,
>
> I propose to replace Map with List in getAll and invokeAll, and
> Iterable in putAll APIs of Ignite 3.x KeyValueView.
>
> 1. Performance
> putAll simply iterates over the map, we can easily accept Iterable instead.
> Iterable can be implemented over anything, it can lazily read data from a
> file or some other place, instead of allocating a huge collection and
> performing unnecessary hashing.
>
> getAll returns a Map, but we don't know if the user code needs a map or
> just wants to iterate over the results, in which case Map is just overhead.
>

"allocating a huge collection" -
This method is not intended for loading a huge number of entries.
The allowed map size for the putAll should be limited to some reasonable
value.

Streaming loading should be addressed in a separate API similar to
DataStream from Ignite 2.


>
> 2. Equality
> getAll returns Map, but in many cases, the map will be useless
> because K does not have proper equals()/hashCode() implementation, so
> map.get(key) does not work.
>

We shouldn't rely on user equals/hashCode while working with key-value API.
Internally it uses binary representation of a user object for comparison
purposes.
The returned map implementation should work in the same way.


>
> Notes:
> - It is not clear which Pair class to use yet - IgniteBiTuple or something
> else.
> - Ignite 3 won't deadlock due to putAll entry order, so we don't have to
> worry about sorting.
>
> Thoughts, objections?
>


-- 

Best regards,
Alexei Scherbakov


Re: [VOTE] Allow or prohibit usages of the Guava library methods

2021-09-09 Thread Alexei Scherbakov
I've checked Guava's feature list and came to a conclusion it's usefulness
has been diminished by switching to base java 11.

-1 for general use, but we can use some code parts then needed.





ср, 8 сент. 2021 г. в 14:09, Вячеслав Коптилин :

> -1
> I am leaning toward -1 because of vulnerability issues (that is a possible
> case in general).
>
> Thanks,
> S.
>
> ср, 8 сент. 2021 г. в 12:13, Andrey Mashenkov  >:
>
> > -1
> > Supporting few copy-pasted methods is much easier than support
> dependencies
> > compatibility.
> >
> > On Tue, Sep 7, 2021 at 7:42 PM Zhenya Stanilovsky
> >  wrote:
> >
> > >
> > > Aleksandr, thanks for this activity.
> > > -1 from my side, all my decisions are in linked discussion.
> > >
> > > >Dear Igniters,
> > > >
> > > >In this thread
> > > ><
> > >
> >
> https://lists.apache.org/thread.html/r4120a03a2bf32098e54e21ae02e509b0d68f413bc7cc1f8f6d85c93d%40%3Cdev.ignite.apache.org%3E
> > > >
> > > >we've been discussing the problems and opportunities of using Guava
> > > >< https://github.com/google/guava > in Ignite 3. We have agreed that
> it
> > > >should be added as a shaded dependency, but we haven't decided whether
> > to
> > > >allow using Guava methods in the Ignite codebase or not. Therefore I
> > would
> > > >like to propose a vote:
> > > >
> > > >[+1 Allow]: allow using Guava methods, if justified.
> > > >[-1 Prohibit]: prohibit using all Guava methods.
> > > >
> > > >The voting will commence on Monday, September 13th at 9:00 UTC. Also
> > feel
> > > >free to express your opinion in the original discussion thread.
> > > >
> > > >--
> > > >With regards,
> > > >Aleksandr Polovtcev
> > >
> > >
> > >
> > >
> >
> >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
> >
>


-- 

Best regards,
Alexei Scherbakov


Re: [Announcement] Apache Ignite 2.11 Code Freeze started

2021-09-09 Thread Maxim Muzafarov
Folks, Denis,


I disagree with you that the changes are ready. They are not compiling
and require additional manual tests. I see the following reasons:
- The major version of dependencies are changed;
- Handling of error codes output changed;
- For a new module developed in 2021 the old version of dependency was
used 12.0.0 (Oct, 2019) instead of the new one available 12.11.0 (Apr,
2021). I don't think this was done unintentionally and additional
investigation is required.
- The patch [1] to fix the build is ready more than 1 week ago and we
still don't have a maintainer for it who will lead this change.

I'd like to mention also that the master branch build and the release
branch build are still BROKEN.


Denis,

I'd like to unblock the 2.11 release pipeline and continue with
preparing the RC taking into account the azure integration is also
important for Apache Ignite users. The other changes are also waiting
to be released for a long time.

So, here is my plan:
- I'll rollback the changes [1] from the release branch.
- I'll merge [1] to the master branch to fix the build.
- Currently, I've limited free time, but I'll try to check the changes
[1] in the real azure environment by myself (probably at the end of
October).
- I'll write a letter with your support on behalf of PMCs to the
Microsoft Azure support and do my best to find some free
resources/credits for our open-source CI project.
- I'll initiate a discussion about moving the AWZ, GCE, Azure modules
to the ignite-extensions project and I hope the community will support
this activity (after all release modules under extensions if need).
- Depending on the discussion with the Azure team, the TC
configuration is needed, so I think your help or the help of someone
from the community will be required here (to run at least basic
suites).

WDYT?
Can we proceed with the release?

[1] https://issues.apache.org/jira/browse/IGNITE-15388

On Tue, 7 Sept 2021 at 18:59, Denis Magda  wrote:
>
> Maxim,
>
> I've found an original ticket where Ilya was helping with review and
> testing of the Azure IP finder:
> https://issues.apache.org/jira/browse/IGNITE-14346
> As I understand, the ticket you're referring to bumps up the version and
> introduces some other little changes:
> https://issues.apache.org/jira/browse/IGNITE-15388
>
> @Vladimir Pligin , could you please help with the
> test in a real Azure environment? See IGNITE-15388
>
> -
> Denis
>
> On Tue, Sep 7, 2021 at 10:46 AM Denis Magda  wrote:
>
> > Maxim,
> >
> > I don't think we're rushing to release the changes. It seems to me that
> > the integration is ready and can be released to the app developers. If we
> > put it off until the next release to add an automated test in the real
> > environment, it can take an unpredictable amount of time before it gets
> > released (someone needs to contribute the test and figure the Azure
> > sponsorship matter).
> >
> > Atri, can you please deploy a 2-node cluster in Azure on separate virtual
> > machines to validate that the IP finder works in this configuration? This
> > is a typical usage scenario for the IP finder - multiple node instances
> > span multiple VMs. If this works, then I would release the contribution.
> >
> > -
> > Denis
> >
> > On Tue, Sep 7, 2021 at 10:01 AM Maxim Muzafarov  wrote:
> >
> >> Denis,
> >>
> >> AWS has a promotional credits program for open-source projects [1],
> >> maybe Azure has something similar too. Have we checked this topic at
> >> least?
> >> I'm not against the changes themselves, they are important for sure.
> >> However, for me, such changes are not as simple as they seem at first
> >> glance and require some time-consumption additional resources and
> >> configurations. I'm glad to provide any help for releasing these
> >> changes but I don't understand why we are rushing with them. And
> >> please don't take words too harshly maybe I just missing some details.
> >>
> >>
> >> [1]
> >> https://aws.amazon.com/blogs/opensource/aws-promotional-credits-open-source-projects/
> >>
> >> On Tue, 7 Sept 2021 at 16:20, Denis Magda  wrote:
> >> >
> >> > Maxim,
> >> >
> >> > What's is the "standard way" and who is going to implement this standard
> >> > approach for the Azure module? Just curious about the ETA of this
> >> feature
> >> > availability. Ignite developers have been requesting the capability for
> >> a
> >> > while and here it is. We're blocking it because of a missing automated
> >> test
> >> > for a real environment. Who is going to develop those tests and, more
> >> > importantly, sponsor Azure resources?
> >> >
> >> > -
> >> > Denis
> >> >
> >> > On Tue, Sep 7, 2021 at 8:03 AM Maxim Muzafarov 
> >> wrote:
> >> >
> >> > > Denis,
> >> > >
> >> > > Thank you for sharing the details. They look great!
> >> > >
> >> > > Can you clarify what is the reason for rushing these changes? Why we
> >> > > should merge changes without test coverage in the 2.11 release without
> >> > > doing it a standard way? Please, don't say that w

Re: Release of pyignite 0.5.2 proposal

2021-09-09 Thread Maxim Muzafarov
+1

On Thu, 9 Sept 2021 at 14:08, Nikolay Izhikov  wrote:
>
> +1 to release ASAP.
>
> > 9 сент. 2021 г., в 13:43, Ivan Daschinsky  написал(а):
> >
> > TC build of release branch --
> > https://tc.sbt-ignite-dev.ru/buildConfiguration/IgniteThinClients_Tests_ThinClientPython/6130289
> >
> > чт, 9 сент. 2021 г. в 13:40, Ivan Daschinsky :
> >
> >> Hi, folks.
> >>
> >> Unfortunately, a quite severe bug found in pyignite since 0.4.0 was found
> >> [1]. Fortunately, it is already fixed. Also there are also a few bugs
> >> already fixed.
> >>
> >> I propose to prepare the next minor release. Branch was already cut and
> >> commits were cherry-picked [3].
> >>
> >> If there are no objections, I will prepare a release and will start voting
> >> thread in a day or two.
> >>
> >> [1] -- https://issues.apache.org/jira/browse/IGNITE-15479
> >> [2] --
> >> https://issues.apache.org/jira/issues/?jql=project%20%3D%20IGNITE%20AND%20fixVersion%20%3D%20python-0.5.2
> >> [3] --
> >> https://github.com/apache/ignite-python-thin-client/commits/pyignite-0.5.2
> >>
> >
> >
> > --
> > Sincerely yours, Ivan Daschinskiy
>


Re: [Announcement] Apache Ignite 2.11 Code Freeze started

2021-09-09 Thread Denis Magda
Maxim,

I'm good with your plan. Thanks for sharing the details. Let's follow the
plan and release 2.11 asap.

-
Denis

On Thu, Sep 9, 2021 at 1:02 PM Maxim Muzafarov  wrote:

> Folks, Denis,
>
>
> I disagree with you that the changes are ready. They are not compiling
> and require additional manual tests. I see the following reasons:
> - The major version of dependencies are changed;
> - Handling of error codes output changed;
> - For a new module developed in 2021 the old version of dependency was
> used 12.0.0 (Oct, 2019) instead of the new one available 12.11.0 (Apr,
> 2021). I don't think this was done unintentionally and additional
> investigation is required.
> - The patch [1] to fix the build is ready more than 1 week ago and we
> still don't have a maintainer for it who will lead this change.
>
> I'd like to mention also that the master branch build and the release
> branch build are still BROKEN.
>
>
> Denis,
>
> I'd like to unblock the 2.11 release pipeline and continue with
> preparing the RC taking into account the azure integration is also
> important for Apache Ignite users. The other changes are also waiting
> to be released for a long time.
>
> So, here is my plan:
> - I'll rollback the changes [1] from the release branch.
> - I'll merge [1] to the master branch to fix the build.
> - Currently, I've limited free time, but I'll try to check the changes
> [1] in the real azure environment by myself (probably at the end of
> October).
> - I'll write a letter with your support on behalf of PMCs to the
> Microsoft Azure support and do my best to find some free
> resources/credits for our open-source CI project.
> - I'll initiate a discussion about moving the AWZ, GCE, Azure modules
> to the ignite-extensions project and I hope the community will support
> this activity (after all release modules under extensions if need).
> - Depending on the discussion with the Azure team, the TC
> configuration is needed, so I think your help or the help of someone
> from the community will be required here (to run at least basic
> suites).
>
> WDYT?
> Can we proceed with the release?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-15388
>
> On Tue, 7 Sept 2021 at 18:59, Denis Magda  wrote:
> >
> > Maxim,
> >
> > I've found an original ticket where Ilya was helping with review and
> > testing of the Azure IP finder:
> > https://issues.apache.org/jira/browse/IGNITE-14346
> > As I understand, the ticket you're referring to bumps up the version and
> > introduces some other little changes:
> > https://issues.apache.org/jira/browse/IGNITE-15388
> >
> > @Vladimir Pligin , could you please help with the
> > test in a real Azure environment? See IGNITE-15388
> >
> > -
> > Denis
> >
> > On Tue, Sep 7, 2021 at 10:46 AM Denis Magda  wrote:
> >
> > > Maxim,
> > >
> > > I don't think we're rushing to release the changes. It seems to me that
> > > the integration is ready and can be released to the app developers. If
> we
> > > put it off until the next release to add an automated test in the real
> > > environment, it can take an unpredictable amount of time before it gets
> > > released (someone needs to contribute the test and figure the Azure
> > > sponsorship matter).
> > >
> > > Atri, can you please deploy a 2-node cluster in Azure on separate
> virtual
> > > machines to validate that the IP finder works in this configuration?
> This
> > > is a typical usage scenario for the IP finder - multiple node instances
> > > span multiple VMs. If this works, then I would release the
> contribution.
> > >
> > > -
> > > Denis
> > >
> > > On Tue, Sep 7, 2021 at 10:01 AM Maxim Muzafarov 
> wrote:
> > >
> > >> Denis,
> > >>
> > >> AWS has a promotional credits program for open-source projects [1],
> > >> maybe Azure has something similar too. Have we checked this topic at
> > >> least?
> > >> I'm not against the changes themselves, they are important for sure.
> > >> However, for me, such changes are not as simple as they seem at first
> > >> glance and require some time-consumption additional resources and
> > >> configurations. I'm glad to provide any help for releasing these
> > >> changes but I don't understand why we are rushing with them. And
> > >> please don't take words too harshly maybe I just missing some details.
> > >>
> > >>
> > >> [1]
> > >>
> https://aws.amazon.com/blogs/opensource/aws-promotional-credits-open-source-projects/
> > >>
> > >> On Tue, 7 Sept 2021 at 16:20, Denis Magda  wrote:
> > >> >
> > >> > Maxim,
> > >> >
> > >> > What's is the "standard way" and who is going to implement this
> standard
> > >> > approach for the Azure module? Just curious about the ETA of this
> > >> feature
> > >> > availability. Ignite developers have been requesting the capability
> for
> > >> a
> > >> > while and here it is. We're blocking it because of a missing
> automated
> > >> test
> > >> > for a real environment. Who is going to develop those tests and,
> more
> > >> > importantly, sponsor Azure resources?
> > >> 

Re: Replace Map with List and Iterable in KeyValueView Ignite 3 APIs

2021-09-09 Thread Valentin Kulichenko
How do we handle the "equality" part in 2.x? The same problem exists there
as well, but we still somehow return a Map.

Generally, I like Pavel's ideas, but there are a couple of issues with
them. Firstly, Java developers are really used to maps in this context.
Introducing something else might be confusing - it's a significant risk.
Secondly, there is no standard Pair class, so we'll have to introduce a
custom one. That said, I would not change this API in Java. In other
languages, however, we can consider this.

-Val

On Thu, Sep 9, 2021 at 8:01 AM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:

> Pavel,
>
> I think the current API looks more natural compared to your proposal.
>
> -1  from my side, see comments below.
>
> чт, 9 сент. 2021 г. в 15:38, Pavel Tupitsyn :
>
> > Igniters,
> >
> > I propose to replace Map with List in getAll and invokeAll, and
> > Iterable in putAll APIs of Ignite 3.x KeyValueView.
> >
> > 1. Performance
> > putAll simply iterates over the map, we can easily accept Iterable
> instead.
> > Iterable can be implemented over anything, it can lazily read data from a
> > file or some other place, instead of allocating a huge collection and
> > performing unnecessary hashing.
> >
> > getAll returns a Map, but we don't know if the user code needs a map or
> > just wants to iterate over the results, in which case Map is just
> overhead.
> >
>
> "allocating a huge collection" -
> This method is not intended for loading a huge number of entries.
> The allowed map size for the putAll should be limited to some reasonable
> value.
>
> Streaming loading should be addressed in a separate API similar to
> DataStream from Ignite 2.
>
>
> >
> > 2. Equality
> > getAll returns Map, but in many cases, the map will be useless
> > because K does not have proper equals()/hashCode() implementation, so
> > map.get(key) does not work.
> >
>
> We shouldn't rely on user equals/hashCode while working with key-value API.
> Internally it uses binary representation of a user object for comparison
> purposes.
> The returned map implementation should work in the same way.
>
>
> >
> > Notes:
> > - It is not clear which Pair class to use yet - IgniteBiTuple or
> something
> > else.
> > - Ignite 3 won't deadlock due to putAll entry order, so we don't have to
> > worry about sorting.
> >
> > Thoughts, objections?
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>


Re: Replace Map with List and Iterable in KeyValueView Ignite 3 APIs

2021-09-09 Thread Pavel Tupitsyn
Val, Alexei - thanks for your replies.
Let's keep the Map approach then.

Regarding tuple equality - there is another thread [1], please have a look.

https://lists.apache.org/thread.html/r9ed68cd515bffab6df821bbeccb8e44d0e5536000e5e7dd05bd87017%40%3Cdev.ignite.apache.org%3E

On Thu, Sep 9, 2021 at 11:21 PM Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> How do we handle the "equality" part in 2.x? The same problem exists there
> as well, but we still somehow return a Map.
>
> Generally, I like Pavel's ideas, but there are a couple of issues with
> them. Firstly, Java developers are really used to maps in this context.
> Introducing something else might be confusing - it's a significant risk.
> Secondly, there is no standard Pair class, so we'll have to introduce a
> custom one. That said, I would not change this API in Java. In other
> languages, however, we can consider this.
>
> -Val
>
> On Thu, Sep 9, 2021 at 8:01 AM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
>
> > Pavel,
> >
> > I think the current API looks more natural compared to your proposal.
> >
> > -1  from my side, see comments below.
> >
> > чт, 9 сент. 2021 г. в 15:38, Pavel Tupitsyn :
> >
> > > Igniters,
> > >
> > > I propose to replace Map with List in getAll and invokeAll, and
> > > Iterable in putAll APIs of Ignite 3.x KeyValueView.
> > >
> > > 1. Performance
> > > putAll simply iterates over the map, we can easily accept Iterable
> > instead.
> > > Iterable can be implemented over anything, it can lazily read data
> from a
> > > file or some other place, instead of allocating a huge collection and
> > > performing unnecessary hashing.
> > >
> > > getAll returns a Map, but we don't know if the user code needs a map or
> > > just wants to iterate over the results, in which case Map is just
> > overhead.
> > >
> >
> > "allocating a huge collection" -
> > This method is not intended for loading a huge number of entries.
> > The allowed map size for the putAll should be limited to some reasonable
> > value.
> >
> > Streaming loading should be addressed in a separate API similar to
> > DataStream from Ignite 2.
> >
> >
> > >
> > > 2. Equality
> > > getAll returns Map, but in many cases, the map will be useless
> > > because K does not have proper equals()/hashCode() implementation, so
> > > map.get(key) does not work.
> > >
> >
> > We shouldn't rely on user equals/hashCode while working with key-value
> API.
> > Internally it uses binary representation of a user object for comparison
> > purposes.
> > The returned map implementation should work in the same way.
> >
> >
> > >
> > > Notes:
> > > - It is not clear which Pair class to use yet - IgniteBiTuple or
> > something
> > > else.
> > > - Ignite 3 won't deadlock due to putAll entry order, so we don't have
> to
> > > worry about sorting.
> > >
> > > Thoughts, objections?
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>