Re: IEP-83 Thin Client Keepalive (heartbeat)

2022-02-04 Thread Maksim Timonin
Hi Pavel,

Thanks for starting this thread! Can I ask some questions here to get the
feature more clearly?

As I understand it correctly, half-state is a possible situation when an
Ignite node goes down or somehow removes connection to a thin client. But
with enabled (true by default) partitionAwareness feature clients can be
notified about topology changes. So, there are possible cases:
1. ThinClient connects to a single node.
2. Ignite node removes connection from itself.

I like the idea for the case with a single node, as it helps fail fast. But
is it OK to connect a client to a single node only?

For the second one: you mention that a case for the second option is
"Long-living and mostly idle connections are especially susceptible to this
behavior". If I understand correctly the connections are removed on the
server side by client idle timeout. Can we just configure the idle timeout
for cases where we really need keeping alive idle connections? Are there
any other cases with unexpectedly dropped connections?

I'm wondering is it OK to keep such connections alive for a long time? Also
in the case of partition awareness features it can lead to wasting TCP
sockets on Ignite nodes, can't it?

Thanks!






On Thu, Feb 3, 2022 at 2:24 PM Pavel Tupitsyn  wrote:

> Igniters,
>
> Please review the proposal to add heartbeat messages to the thin client
> protocol (both 2.x and 3.x) and let me know your thoughts:
>
>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-83+Thin+Client+Keepalive
>


Re: [DISCUSSION] Shmem removal.

2022-02-04 Thread Ivan Daschinsky
https://issues.apache.org/jira/browse/IGNITE-16480 -- I've filed ticked.

пт, 7 янв. 2022 г. в 23:44, Valentin Kulichenko <
valentin.kuliche...@gmail.com>:

> Doesn't look like there are any objections - it's been a month since you
> started this thread. Let's create a ticket.
>
> -Val
>
>
> On Thu, Jan 6, 2022 at 1:22 AM Ivan Daschinsky 
> wrote:
>
> > Hi, Val. My plan was to file a specific ticket after discussion. If the
> > community agrees that this module should be removed, I will file a
> specific
> > ticket for it.
> >
> > ср, 5 янв. 2022 г., 22:26 Valentin Kulichenko <
> > valentin.kuliche...@gmail.com
> > >:
> >
> > > Hi Ivan,
> > >
> > > Do we have a ticket for this?
> > >
> > > -Val
> > >
> > > On Fri, Dec 3, 2021 at 10:58 AM Valentin Kulichenko <
> > > valentin.kuliche...@gmail.com> wrote:
> > >
> > > > I think we can safely remove it.
> > > >
> > > > -Val
> > > >
> > > > On Thu, Dec 2, 2021 at 11:52 PM Ivan Daschinsky  >
> > > > wrote:
> > > >
> > > >> Hi, igniters.
> > > >>
> > > >> Recently I've discovered one fact
> > > >> 1. GridShmemCommunicationClient and all shmem functionality are
> broken
> > > >> since 2.10. In master it is broken since August 2020. And nobody
> have
> > > >> noticed it, only one thread in user list.
> > > >> 2. We have source code for native JNI library (that is shipped in
> > > >> ignite-shmem.jar), but we never built it since 2015.
> > > >> 3. This code is of questionable quality, contains outdated internal
> > gcc
> > > >> api
> > > >> (__sync builtins, now deprecated in favour of __atomic builtins in
> gcc
> > > and
> > > >> is not advisable to use since C++ 11). It contains a lot of autotool
> > > mess,
> > > >> while just one CMakeFile.txt is enough to build the same
> > > >> 4. This code doesn't even compile on modern gcc (gcc 9.3.0 namely)
> > > >>
> > > >> We have 2 options
> > > >> 1. If this concept is useful, we (for example I can do it) should
> > > rewrite
> > > >> native part,
> > > >> a. Use C++ 11 and header-only boost.interprocess [1]
> > > >> b. Build it regularly with CMake and incorporate build in regular TC
> > > runs
> > > >> (via maven-cmake-plugin,
> > > >> see for example my numa-allocator [2]).
> > > >> 2. If this concept and functionality is not useful, we should remove
> > it,
> > > >> may be even in 2.12
> > > >>
> > > >>
> > > >> [1] --
> > https://www.boost.org/doc/libs/1_77_0/doc/html/interprocess.html
> > > >> [2] --
> > > >>
> > > >>
> > >
> >
> https://github.com/apache/ignite/pull/9569/files#diff-77baf2378aa83911a8c3091814db3ff60b7bf328c4ab4850f707717ed96f3d92
> > > >> --
> > > >> Sincerely yours, Ivan Daschinskiy
> > > >>
> > > >
> > >
> >
>


-- 
Sincerely yours, Ivan Daschinskiy


Re: Travis service is not working properly

2022-02-04 Thread Anton Vinogradov
>> Should I add this info for the INFRA ticket?
Please do

On Fri, Feb 4, 2022 at 1:52 PM Maksim Timonin 
wrote:

> Hi, I have a similar issue for my PR [1]:
>
> 1. Github shows that the previous commit check is not completed yet. But on
> Travis' side I see that it actually has already finished [2].
> 2. There is a new commit after the previous and Travis ignores it and
> doesn't schedule a new build.
>
> Should I add this info for the INFRA ticket?
>
> [1] https://github.com/apache/ignite/pull/9788
> [2] https://app.travis-ci.com/github/apache/ignite/builds/245860409
>
>
>
> On Wed, Feb 2, 2022 at 5:42 PM Anton Vinogradov  wrote:
>
> > Asked the INFRA [1]
> >
> > [1] https://issues.apache.org/jira/browse/INFRA-22827
> >
> > On Wed, Feb 2, 2022 at 5:29 PM Anton Vinogradov  wrote:
> >
> > > Igniters,
> > >
> > > Seems the Travis service is not working properly.
> > > For example, it never checked my PR [1], any ideas why?
> > >
> > > Who knows how to fix it?
> > >
> > > [1] https://github.com/apache/ignite/pull/9661
> > >
> >
>


Re: Travis service is not working properly

2022-02-04 Thread Maksim Timonin
Hi, I have a similar issue for my PR [1]:

1. Github shows that the previous commit check is not completed yet. But on
Travis' side I see that it actually has already finished [2].
2. There is a new commit after the previous and Travis ignores it and
doesn't schedule a new build.

Should I add this info for the INFRA ticket?

[1] https://github.com/apache/ignite/pull/9788
[2] https://app.travis-ci.com/github/apache/ignite/builds/245860409



On Wed, Feb 2, 2022 at 5:42 PM Anton Vinogradov  wrote:

> Asked the INFRA [1]
>
> [1] https://issues.apache.org/jira/browse/INFRA-22827
>
> On Wed, Feb 2, 2022 at 5:29 PM Anton Vinogradov  wrote:
>
> > Igniters,
> >
> > Seems the Travis service is not working properly.
> > For example, it never checked my PR [1], any ideas why?
> >
> > Who knows how to fix it?
> >
> > [1] https://github.com/apache/ignite/pull/9661
> >
>


Re: Unmarshalable result from the MX beans

2022-02-04 Thread Maksim Timonin
Hi Vladimir,

I think that var 3. is OK. Currently `clusterStates` javadocs declares
`IgniteCheckedException` but actually throws `IgniteException`. So this is
a bug in any case, so I think we can just replace it with JMException.

I'd like to propose to use the same approach for other JMX methods too:
`undeployTaskFromGrid`, `executeTask`, `pingNodeByAddress`.

I think it's safe to use only String messages instead of full
IgniteException as the cause. Don't think that JMX bean should return full
stack trace of Ignite cluster exception.


On Thu, Feb 3, 2022 at 9:54 PM Vladimir Steshin  wrote:

>  Hi, Igniters.
>
>  We've found that invocation results from some MX beans might not be
> properly represented on clients like jconsole [1]. In the exception
> cases. We pass Ignite exceptions or JMException with Ignite exception
> causes to the client. And the client fails to unmarshal unknown
> exceptions instead of showing failure reason.
>
> Examples:
>
>   * /IgniteMXBean.clusterState()/ can throw /IgniteException/
>   * /IgniteMXBean.executeTask()/ can throw /JMException/ holding
> /IgniteException/
>   * /IgniteClusterMXBean.tag()/ can throw /JMException/ holding
> /IgniteException/
>
>
> Solutions might be:
>
>  1. For existing /JMExceptions/ let's keep only the error message and
> hold no cause which can't be deserialized.
>  2. #1 + Let's throw just /RuntimeException/ with the error message
> instead of /IgniteException/ (for example in
> /IgniteMXBean.clusterState()/).
>  3. #1 + Let's use and declare similar /'throws JMException'/ at methods
> like /IgniteMXBean.clusterState()/. But the user code that uses
> mx-bean public API might not be compiled with this change.
>  4. Deprecate these exception-throwing methods and bring new ones
> returning string-result like "OK" or "failed: cause". But
> /IgniteMXBean/ has already had several change-cluster-state methods
> including deprecated.
>
>
> My suggestion is #3. WDYT?
>
>
> [1] https://issues.apache.org/jira/browse/IGNITE-16416
>
>