Re: Re[4]: [DISCUSSION] Fail on non-colocated join
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 : > > > 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 < > arzamas...@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 < > >> arzamas...@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
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 < arzamas...@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 < >> arzamas...@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
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 : > > > > > >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 < > arzamas...@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[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 < arzamas...@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: [DISCUSSION] Fail on non-colocated join
Hello! I think that we should issue a developer warning with LT.warn. I don't think we should raise an exception. Our code may not have enough information. For example, it is possible that the data is collocated via constraint observed by user, such as, ina table of key (id, affKey) there is join using (affVal), which looks like it requires a distributed join, but the user observes that affKey == affVal so it's actually OK. If it breaks existing working code it may not be done that way. Regards, -- Ilya Kasnacheev ср, 3 февр. 2021 г. в 09:05, Zhenya Stanilovsky : > > > 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: [DISCUSSION] Fail on non-colocated join
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: [DISCUSSION] Fail on non-colocated join
I think it should be the first option - just throw a clear exception if a query with a non-colocated join is executed without the flag set. Explicitly turning this mode on seems wrong, because in most cases this is not what a user intends to do. We also should make sure to run performance tests. As far as I know, this was not implemented initially because the assumption was that performing such a check on every single query might have a significant negative impact, especially for OLTP patterns that require low latency and high throughput. -Val On Tue, Feb 2, 2021 at 7:18 AM Max Timonin wrote: > 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 >
[DISCUSSION] Fail on non-colocated join
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
