You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Zhenya Stanilovsky <ar...@mail.ru.INVALID> on 2021/02/03 11:48:24 UTC

Re[2]: [DISCUSSION] Fail on non-colocated join



  
>If it breaks existing working code it may not be done that way.
 
Who reads the logs ? 
Is it violates apache way approach or some existing rules ?
 
thanks !
 
  
>Regards,
>--
>Ilya Kasnacheev
>
>
>ср, 3 февр. 2021 г. в 09:05, Zhenya Stanilovsky < arzamas123@mail.ru.invalid
>>:
>
>>
>>
>> Maxim it`s cool that it`s moved :)
>> +1 for exception, but take into account such use case :
>> T1 (country, city) affinity_key=country and T2 (country,city)
>> affinity_key=country join with «city» usage — will be correct here (i hope,
>> need to recheck of course) thus seems you must give some flag\hint what
>> ever to run such reqs.
>>
>> thanks !
>>
>> >Hi, Igniters!
>> >
>> >Last week I investigated a bug [1]. It's about an incorrect result for
>> >non-colocated joins. For such joins it's required to set up the
>> >"distributedJoin" flag, or try to make joined tables colocated. It is
>> >covered in docs [2]. But it's not obvious and some users don't read that
>> or
>> >forget about that. In result there are wrong results for some queries that
>> >are pretty hard to debug.
>> >
>> >There is a ticket [3] with a comment, where it's suggested to add a check
>> >for such joins. I tried to implement it and found a place where it's
>> >possible to put this check. But there is an open question what this check
>> >should do. Currently I see 2 ways for that:
>> >1. Forbid non-colocated joins that aren't marked with the distributedJoin
>> >flag, and throw an exception.
>> >2. Check every query for such joins and implicitly setup a distributedJoin
>> >flag for them.
>> >
>> >Both solutions may break compatibility, but is this compatibility OK?
>> >
>> >Igniters, what do you think?
>> >
>> >[1]  https://issues.apache.org/jira/browse/IGNITE-12847
>> >[2]
>> >
>>  https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins
>> >[3]  https://issues.apache.org/jira/browse/IGNITE-13019
>>
>>
>>
>> 
 
 
 
 

Re: Re[4]: [DISCUSSION] Fail on non-colocated join

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

I don't think this is absolutely needed. From my userlist experience I
don't get particularly many questions about distributed joins. They are,
but it's not a hot topic, which is somewhat surprising to me actually. I
would expect more people struggling with them.

To tell people about erroneous places in the code, LT developer warning is
an optimal fit. If we see that many people start asking about this, we may
reconsider.

In my opinion, even changing `distributedJoins' default to true is less of
a breaking change.

Regards,
-- 
Ilya Kasnacheev


ср, 3 февр. 2021 г. в 16:49, Zhenya Stanilovsky <arzamas123@mail.ru.invalid
>:

>
>
> I think it`s « absolutely needed » case ) Many people will find erroneous
> places.
>
>
>
> >Hello!
> >
> >Many people do read logs, especially developers. You would be amazed how
> >many people come to discuss even the most benign of warnings.
> >
> >I think it violates the Apache Ignite compatibility policy, where we do
> not
> >break existing code during a major release.
> >
> >We do not always hold on to that principle, for example Baseline Topology
> >introduced some changes which needed to be explicitly handled by the user.
> >But in this case, it looks like a violation.
> >
> >https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist
> >
> >It's checklist, 1b.
> >
> >Regards,
> >--
> >Ilya Kasnacheev
> >
> >
> >ср, 3 февр. 2021 г. в 14:48, Zhenya Stanilovsky <
> arzamas123@mail.ru.invalid
> >>:
> >
> >>
> >>
> >>
> >>
> >> >If it breaks existing working code it may not be done that way.
> >>
> >> Who reads the logs ?
> >> Is it violates apache way approach or some existing rules ?
> >>
> >> thanks !
> >>
> >>
> >> >Regards,
> >> >--
> >> >Ilya Kasnacheev
> >> >
> >> >
> >> >ср, 3 февр. 2021 г. в 09:05, Zhenya Stanilovsky <
> >>  arzamas123@mail.ru.invalid
> >> >>:
> >> >
> >> >>
> >> >>
> >> >> Maxim it`s cool that it`s moved :)
> >> >> +1 for exception, but take into account such use case :
> >> >> T1 (country, city) affinity_key=country and T2 (country,city)
> >> >> affinity_key=country join with «city» usage — will be correct here (i
> >> hope,
> >> >> need to recheck of course) thus seems you must give some flag\hint
> what
> >> >> ever to run such reqs.
> >> >>
> >> >> thanks !
> >> >>
> >> >> >Hi, Igniters!
> >> >> >
> >> >> >Last week I investigated a bug [1]. It's about an incorrect result
> for
> >> >> >non-colocated joins. For such joins it's required to set up the
> >> >> >"distributedJoin" flag, or try to make joined tables colocated. It
> is
> >> >> >covered in docs [2]. But it's not obvious and some users don't read
> >> that
> >> >> or
> >> >> >forget about that. In result there are wrong results for some
> queries
> >> that
> >> >> >are pretty hard to debug.
> >> >> >
> >> >> >There is a ticket [3] with a comment, where it's suggested to add a
> >> check
> >> >> >for such joins. I tried to implement it and found a place where it's
> >> >> >possible to put this check. But there is an open question what this
> >> check
> >> >> >should do. Currently I see 2 ways for that:
> >> >> >1. Forbid non-colocated joins that aren't marked with the
> >> distributedJoin
> >> >> >flag, and throw an exception.
> >> >> >2. Check every query for such joins and implicitly setup a
> >> distributedJoin
> >> >> >flag for them.
> >> >> >
> >> >> >Both solutions may break compatibility, but is this compatibility
> OK?
> >> >> >
> >> >> >Igniters, what do you think?
> >> >> >
> >> >> >[1]  https://issues.apache.org/jira/browse/IGNITE-12847
> >> >> >[2]
> >> >> >
> >> >>
> >>
> https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins
> >> >> >[3]  https://issues.apache.org/jira/browse/IGNITE-13019
> >> >>
> >> >>
> >> >>
> >> >>
> >>
> >>
> >>
> >>
>
>
>
>

Re[4]: [DISCUSSION] Fail on non-colocated join

Posted by Zhenya Stanilovsky <ar...@mail.ru.INVALID>.

I think it`s « absolutely needed » case ) Many people will find erroneous places.


 
>Hello!
>
>Many people do read logs, especially developers. You would be amazed how
>many people come to discuss even the most benign of warnings.
>
>I think it violates the Apache Ignite compatibility policy, where we do not
>break existing code during a major release.
>
>We do not always hold on to that principle, for example Baseline Topology
>introduced some changes which needed to be explicitly handled by the user.
>But in this case, it looks like a violation.
>
>https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist
>
>It's checklist, 1b.
>
>Regards,
>--
>Ilya Kasnacheev
>
>
>ср, 3 февр. 2021 г. в 14:48, Zhenya Stanilovsky < arzamas123@mail.ru.invalid
>>:
>
>>
>>
>>
>>
>> >If it breaks existing working code it may not be done that way.
>>
>> Who reads the logs ?
>> Is it violates apache way approach or some existing rules ?
>>
>> thanks !
>>
>>
>> >Regards,
>> >--
>> >Ilya Kasnacheev
>> >
>> >
>> >ср, 3 февр. 2021 г. в 09:05, Zhenya Stanilovsky <
>>  arzamas123@mail.ru.invalid
>> >>:
>> >
>> >>
>> >>
>> >> Maxim it`s cool that it`s moved :)
>> >> +1 for exception, but take into account such use case :
>> >> T1 (country, city) affinity_key=country and T2 (country,city)
>> >> affinity_key=country join with «city» usage — will be correct here (i
>> hope,
>> >> need to recheck of course) thus seems you must give some flag\hint what
>> >> ever to run such reqs.
>> >>
>> >> thanks !
>> >>
>> >> >Hi, Igniters!
>> >> >
>> >> >Last week I investigated a bug [1]. It's about an incorrect result for
>> >> >non-colocated joins. For such joins it's required to set up the
>> >> >"distributedJoin" flag, or try to make joined tables colocated. It is
>> >> >covered in docs [2]. But it's not obvious and some users don't read
>> that
>> >> or
>> >> >forget about that. In result there are wrong results for some queries
>> that
>> >> >are pretty hard to debug.
>> >> >
>> >> >There is a ticket [3] with a comment, where it's suggested to add a
>> check
>> >> >for such joins. I tried to implement it and found a place where it's
>> >> >possible to put this check. But there is an open question what this
>> check
>> >> >should do. Currently I see 2 ways for that:
>> >> >1. Forbid non-colocated joins that aren't marked with the
>> distributedJoin
>> >> >flag, and throw an exception.
>> >> >2. Check every query for such joins and implicitly setup a
>> distributedJoin
>> >> >flag for them.
>> >> >
>> >> >Both solutions may break compatibility, but is this compatibility OK?
>> >> >
>> >> >Igniters, what do you think?
>> >> >
>> >> >[1]  https://issues.apache.org/jira/browse/IGNITE-12847
>> >> >[2]
>> >> >
>> >>
>>  https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins
>> >> >[3]  https://issues.apache.org/jira/browse/IGNITE-13019
>> >>
>> >>
>> >>
>> >>
>>
>>
>>
>> 
 
 
 
 

Re: Re[2]: [DISCUSSION] Fail on non-colocated join

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

Many people do read logs, especially developers. You would be amazed how
many people come to discuss even the most benign of warnings.

I think it violates the Apache Ignite compatibility policy, where we do not
break existing code during a major release.

We do not always hold on to that principle, for example Baseline Topology
introduced some changes which needed to be explicitly handled by the user.
But in this case, it looks like a violation.

https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist

It's checklist, 1b.

Regards,
-- 
Ilya Kasnacheev


ср, 3 февр. 2021 г. в 14:48, Zhenya Stanilovsky <arzamas123@mail.ru.invalid
>:

>
>
>
>
> >If it breaks existing working code it may not be done that way.
>
> Who reads the logs ?
> Is it violates apache way approach or some existing rules ?
>
> thanks !
>
>
> >Regards,
> >--
> >Ilya Kasnacheev
> >
> >
> >ср, 3 февр. 2021 г. в 09:05, Zhenya Stanilovsky <
> arzamas123@mail.ru.invalid
> >>:
> >
> >>
> >>
> >> Maxim it`s cool that it`s moved :)
> >> +1 for exception, but take into account such use case :
> >> T1 (country, city) affinity_key=country and T2 (country,city)
> >> affinity_key=country join with «city» usage — will be correct here (i
> hope,
> >> need to recheck of course) thus seems you must give some flag\hint what
> >> ever to run such reqs.
> >>
> >> thanks !
> >>
> >> >Hi, Igniters!
> >> >
> >> >Last week I investigated a bug [1]. It's about an incorrect result for
> >> >non-colocated joins. For such joins it's required to set up the
> >> >"distributedJoin" flag, or try to make joined tables colocated. It is
> >> >covered in docs [2]. But it's not obvious and some users don't read
> that
> >> or
> >> >forget about that. In result there are wrong results for some queries
> that
> >> >are pretty hard to debug.
> >> >
> >> >There is a ticket [3] with a comment, where it's suggested to add a
> check
> >> >for such joins. I tried to implement it and found a place where it's
> >> >possible to put this check. But there is an open question what this
> check
> >> >should do. Currently I see 2 ways for that:
> >> >1. Forbid non-colocated joins that aren't marked with the
> distributedJoin
> >> >flag, and throw an exception.
> >> >2. Check every query for such joins and implicitly setup a
> distributedJoin
> >> >flag for them.
> >> >
> >> >Both solutions may break compatibility, but is this compatibility OK?
> >> >
> >> >Igniters, what do you think?
> >> >
> >> >[1]  https://issues.apache.org/jira/browse/IGNITE-12847
> >> >[2]
> >> >
> >>
> https://ignite.apache.org/docs/latest/SQL/distributed-joins#distributed-joins
> >> >[3]  https://issues.apache.org/jira/browse/IGNITE-13019
> >>
> >>
> >>
> >>
>
>
>
>