Re: IEP-68: Thin Client Data Streamer
Igniters,
I've implemented a "stateful" approach (see IEP) in .NET Thin Client:
client-side flush adds data to server-side streamer, but does not flush it
immediately.
It provides ~35% perf improvement over stateless approach (where every
client request opens a new streamer, adds data, closes streamer).
However, in this case we lose server-side buffered data if the server node
fails, which seems to be a serious issue.
On the client we don't even know which part of data was lost, so we can't
re-send this data.
Therefore, a stateless approach seems to be the only proper way:
once a client request completes, the data is guaranteed to be in the cache,
and failed requests (connection loss, node failure) can be retried.
This simplifies both client and server implementations.
Thoughts?
On Tue, Mar 9, 2021 at 6:32 PM Pavel Tupitsyn wrote:
> Alex,
>
> I did not include this for two reasons:
>
> 1) Existing thick API behavior is very confusing and counter-intuitive:
> it returns a Future that won't complete unless you call more APIs.
>
> I've seen multiple users doing this:
> await streamer.Add(1, 1);
> await streamer.Add(2, 2);
>
> This code hangs - the first Add will never complete, because the batch is
> not sent,
> and the batch won't be sent because we are awaiting the first Add.
>
> 2) I'm not sure what's the use case for this feature - why do we want to
> know
> when the flush happens?
>
> We can come up with a better API, but should we bother at all?
>
> On Fri, Mar 5, 2021 at 6:09 PM Alex Plehanov
> wrote:
>
>> Pavel,
>>
>> IEP looks good to me overall. I have only one concern: currently, for
>> thick
>> client "add" method we return the future which completes when the current
>> batch is actually added to the cache, even if "flush" method is not
>> invoked
>> explicitly. For thin client with the proposed protocol we can't provide
>> such functionality without explicit "flush". Perhaps we should notify
>> client when batch actually flushed and send this notification when some
>> "require notification" flag is sent with the request. WDYT?
>>
>> пт, 5 мар. 2021 г. в 17:03, Ivan Daschinsky :
>>
>> > Agree, this is completely offtopic here.
>> >
>> > пт, 5 мар. 2021 г. в 17:02, Pavel Tupitsyn :
>> >
>> > > > custom DSL
>> > > This is a topic for 3.0 - would you like to start a separate thread?
>> > >
>> > > On Fri, Mar 5, 2021 at 4:54 PM Ivan Daschinsky
>> > > wrote:
>> > >
>> > > > I suppose, that the best variantl -- custom DSL similar to MongoDB
>> > query
>> > > > language.
>> > > > I understand that implementing this is much harder, but users want
>> this
>> > > > variant.
>> > > >
>> > > > пт, 5 мар. 2021 г. в 16:52, Pavel Tupitsyn :
>> > > >
>> > > > > > I suppose this is of questional usability, same for existing
>> > > > > > implementation for ScanQuery and ContinousQuery
>> > > > >
>> > > > > One way or another, we need to be able to run custom logic
>> > > > > on the server side from thin clients.
>> > > > >
>> > > > > Our current direction can be seen as "Java is our DSL":
>> > > > > write server-side logic in Java, deploy to servers, call from thin
>> > > > clients.
>> > > > >
>> > > > > This approach is already used for Services, Compute, ScanQuery,
>> > > > > ContinuousQuery.
>> > > > >
>> > > > > If you have a better idea in mind, please share.
>> > > > >
>> > > > > On Fri, Mar 5, 2021 at 4:12 PM Ivan Daschinsky <
>> ivanda...@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > >>> - Corresponding types should exist on server nodes
>> > > > > > Ok, but I suppose this is of questional usability, same for
>> > existing
>> > > > > > implementation for ScanQuery and ContinousQuery.
>> > > > > > For queries it's probably ok to add some custom filters and put
>> > them
>> > > in
>> > > > > > servers' classpathes, but I can hardly imagine
>> > > > > > somebody wants to create custom Receiver this way.
>> > > > > >
>> > > > > > пт, 5 мар. 2021 г. в 15:54, Pavel Tupitsyn <
>> ptupit...@apache.org>:
>> > > > > >
>> > > > > > > > How do you suggests to serialize this object?
>> > > > > > >
>> > > > > > > Like a normal binary object. This scenario already exists for
>> > > > ScanQuery
>> > > > > > and
>> > > > > > > ContinuousQuery filters.
>> > > > > > > - It works for Java and .NET; potentially for other platforms
>> > > > > > > - Corresponding types should exist on server nodes
>> > > > > > >
>> > > > > > > E.g. if a Java class `MyFilter { String containsText }` is
>> loaded
>> > > on
>> > > > > > server
>> > > > > > > nodes,
>> > > > > > > a Python thin client can easily write a corresponding
>> > BinaryObject
>> > > > with
>> > > > > > one
>> > > > > > > field to achieve server-side filtering.
>> > > > > > >
>> > > > > > >
>> > > > > > > > I also suppose, that closing should be done wia
>> > OP_RESOURCE_CLOSE
>> > > > > > >
>> > > > > > > Thanks, forgot to mention this - updated the IEP.
>> > > > > > >
>> > > > > > >
>> > > > > > > On Fri, Mar 5, 2021 at 3:42 PM Ivan Daschinsky <
>> > > ivand
Re: IEP-68: Thin Client Data Streamer
Pavel, +1 from me for a stateless approach. We could definitely invent something here to make a stateful approach working but that would make this feature too complex and error-prone, IMO. It makes sense to file an improvement ticket with benchmark results and maybe code draft if we decide to move this way in future though. Best Regards, Igor On Tue, May 18, 2021 at 11:47 AM Pavel Tupitsyn wrote: > Igniters, > > I've implemented a "stateful" approach (see IEP) in .NET Thin Client: > client-side flush adds data to server-side streamer, but does not flush it > immediately. > It provides ~35% perf improvement over stateless approach (where every > client request opens a new streamer, adds data, closes streamer). > > However, in this case we lose server-side buffered data if the server node > fails, which seems to be a serious issue. > On the client we don't even know which part of data was lost, so we can't > re-send this data. > > Therefore, a stateless approach seems to be the only proper way: > once a client request completes, the data is guaranteed to be in the cache, > and failed requests (connection loss, node failure) can be retried. > This simplifies both client and server implementations. > > Thoughts? > > On Tue, Mar 9, 2021 at 6:32 PM Pavel Tupitsyn > wrote: > > > Alex, > > > > I did not include this for two reasons: > > > > 1) Existing thick API behavior is very confusing and counter-intuitive: > > it returns a Future that won't complete unless you call more APIs. > > > > I've seen multiple users doing this: > > await streamer.Add(1, 1); > > await streamer.Add(2, 2); > > > > This code hangs - the first Add will never complete, because the batch is > > not sent, > > and the batch won't be sent because we are awaiting the first Add. > > > > 2) I'm not sure what's the use case for this feature - why do we want to > > know > > when the flush happens? > > > > We can come up with a better API, but should we bother at all? > > > > On Fri, Mar 5, 2021 at 6:09 PM Alex Plehanov > > wrote: > > > >> Pavel, > >> > >> IEP looks good to me overall. I have only one concern: currently, for > >> thick > >> client "add" method we return the future which completes when the > current > >> batch is actually added to the cache, even if "flush" method is not > >> invoked > >> explicitly. For thin client with the proposed protocol we can't provide > >> such functionality without explicit "flush". Perhaps we should notify > >> client when batch actually flushed and send this notification when some > >> "require notification" flag is sent with the request. WDYT? > >> > >> пт, 5 мар. 2021 г. в 17:03, Ivan Daschinsky : > >> > >> > Agree, this is completely offtopic here. > >> > > >> > пт, 5 мар. 2021 г. в 17:02, Pavel Tupitsyn : > >> > > >> > > > custom DSL > >> > > This is a topic for 3.0 - would you like to start a separate thread? > >> > > > >> > > On Fri, Mar 5, 2021 at 4:54 PM Ivan Daschinsky > > >> > > wrote: > >> > > > >> > > > I suppose, that the best variantl -- custom DSL similar to MongoDB > >> > query > >> > > > language. > >> > > > I understand that implementing this is much harder, but users want > >> this > >> > > > variant. > >> > > > > >> > > > пт, 5 мар. 2021 г. в 16:52, Pavel Tupitsyn >: > >> > > > > >> > > > > > I suppose this is of questional usability, same for existing > >> > > > > > implementation for ScanQuery and ContinousQuery > >> > > > > > >> > > > > One way or another, we need to be able to run custom logic > >> > > > > on the server side from thin clients. > >> > > > > > >> > > > > Our current direction can be seen as "Java is our DSL": > >> > > > > write server-side logic in Java, deploy to servers, call from > thin > >> > > > clients. > >> > > > > > >> > > > > This approach is already used for Services, Compute, ScanQuery, > >> > > > > ContinuousQuery. > >> > > > > > >> > > > > If you have a better idea in mind, please share. > >> > > > > > >> > > > > On Fri, Mar 5, 2021 at 4:12 PM Ivan Daschinsky < > >> ivanda...@gmail.com> > >> > > > > wrote: > >> > > > > > >> > > > > > >>> - Corresponding types should exist on server nodes > >> > > > > > Ok, but I suppose this is of questional usability, same for > >> > existing > >> > > > > > implementation for ScanQuery and ContinousQuery. > >> > > > > > For queries it's probably ok to add some custom filters and > put > >> > them > >> > > in > >> > > > > > servers' classpathes, but I can hardly imagine > >> > > > > > somebody wants to create custom Receiver this way. > >> > > > > > > >> > > > > > пт, 5 мар. 2021 г. в 15:54, Pavel Tupitsyn < > >> ptupit...@apache.org>: > >> > > > > > > >> > > > > > > > How do you suggests to serialize this object? > >> > > > > > > > >> > > > > > > Like a normal binary object. This scenario already exists > for > >> > > > ScanQuery > >> > > > > > and > >> > > > > > > ContinuousQuery filters. > >> > > > > > > - It works for Java and .NET; potentially for other > platforms > >> > > > > > > - Corresponding types sho
Re: [DISCUSSION] MaxLineLength checkstyle rule
Hello. > BTW, was there any progress with this? Patch is ready, please, join the review - https://github.com/apache/ignite/pull/9106 > 10 мая 2021 г., в 09:47, Ivan Pavlukhin написал(а): > > +1 for 140 lines compromise. > +1 if someone is ready to fix everything to fit 120. > > BTW, was there any progress with this? > > 2021-04-15 21:41 GMT+03:00, Maxim Muzafarov : >> Folks, >> >> I've briefly checked the total amount of the max length violations: >> >> 120 line length - 5540 violations >> 130 line length - 1891 violations >> 140 line length - 895 violations >> 150 line length - 478 violations >> >> >> I think the 140 max line length might be the best option for us. >> >> On Thu, 15 Apr 2021 at 14:51, Ivan Daschinsky wrote: >>> >>> But super long lines are a real problem while merging. It is super >>> inconvenient. >>> 120 chars is a good compromise. >>> >>> чт, 15 апр. 2021 г. в 14:39, Zhenya Stanilovsky >>> >>> : >>> Python is not so verbose as java ) +1 for 140 > Hi! > Personally, I suppose that 120 chars per line is OK. Moreover, many > codestyles suggests less chars per line. > For example PEP8 recommends 80 (but we use 120 in pyignite and flake8 > codestyle checks it). Google java codestyle insists on 100. > > More than 120 chars is too long as for me and is not convenient for > 3-way > merges. > > чт, 15 апр. 2021 г. в 12:28, Nikolay Izhikov < nizhi...@apache.org >: > >> Hello, Ilya. >> >> Thanks for the feedback. >> >> 140 characters is fine for me. >> >>> 15 апр. 2021 г., в 12:25, Ilya Kasnacheev < >>> ilya.kasnach...@gmail.com > >> написал(а): >>> >>> Hello! >>> >>> Please find attached the distribution of line lengths in the >>> project, in >> the form of (count, line length). >>> >>> I think that we can enforce a hard limit of 140 chars per line. I think >> that having longer lines is excessive and does not benefit >> readability. >>> >>> Having a limit of 150 or 180 does not give us much since there's >>> still >> a long tail which has to be fixed. >>> >>> Regards, >>> -- >>> Ilya Kasnacheev >>> >>> >>> чт, 15 апр. 2021 г. в 11:30, Nikolay Izhikov < nizhi...@apache.org : >>> Hello, Igniters. >>> >>> Right now, we have a code style rule [1] - the line should fit in >>> 120 >> characters. >>> But, this rule violated in many and many places through code. >>> I have a plan to add a check style rule to force maximum line >>> length. >>> >>> For me, personally, 120 characters a bit old-fashioned >>> restriction. >>> Should we increase the maximum line length to 150 or even 180 characters? >>> >>> [1] https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines >>> >> >> > -- > Sincerely yours, Ivan Daschinskiy >>> >>> >>> >>> -- >>> Sincerely yours, Ivan Daschinskiy >> > > > -- > > Best regards, > Ivan Pavlukhin
ignite read stale data from backup node
Hi Ignites,
I ran into in-consistency data issues on version 2.8.1. I have three nodes run
as a cluster and the cache configuration as:
CacheConfiguration cacheConfiguration = new
CacheConfiguration<>(Balance.class.getSimpleName());
cacheConfiguration.setIndexedTypes(String.class, Balance.class);
cacheConfiguration.setSqlIndexMaxInlineSize(100);
cacheConfiguration.setSqlSchema("PUBLIC");
cacheConfiguration.setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL);
cacheConfiguration.setCacheMode(CacheMode.PARTITIONED);
cacheConfiguration.setBackups(4);
cacheConfiguration.setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC);
Then I have very simple code to add balance in a loop:
for (int i = 0; i < 1; i++) {
balance = balanceDao.findByKey(accountId, "USD");
balance.setQuantity(balance.getQuantity().add(BigDecimal.ONE));
balanceDao.save(balance);
}
when I run above on the primary node, I always have balance increased 1
correctly, however when I run that on backup node, sometimes my balance
increased around 8k, and sometimes 9k.
if setWriteSynchronizationMode was set to PRIMARY_SYNC and setReadFromBackup
was set to false, I can get correct balance on all nodes.
is this a bug on 2.8.1 or anything wrong with my configuration?
Obsolete classnames.properties
Hello, Igniters! I've recently stumbled across an issue where some tests can't be run locally due to the absence of some entries in "classnames.properties" file. For example, TcpRestUnmarshalVulnerabilityTest can't be run locally because "classnames.properties" lacks org.apache.ignite.configuration.EncryptionConfiguration and org.apache.ignite.configuration.PageReplacementMode. On teamcity, however, such tests pass without any problem, because "classnames.properties" is also generated by org.apache.ignite.tools.classgen.ClassesGenerator during process-classes phase in maven build. I propose removing this file, as it is generated during the build which means it's a pointless job keeping it up-to-date in the repository. Also it's impossible to tell right now if it is or it is not up-to-date (without executing ClassesGenerator). Kind regards, Semyon.
Re: Obsolete classnames.properties
Gmail treated this as spam for me. Bump. 2021-05-18 14:39 GMT+03:00, Данилов Семён : > Hello, Igniters! > > I've recently stumbled across an issue where some tests can't be run locally > due to the absence of some entries in "classnames.properties" file. > For example, TcpRestUnmarshalVulnerabilityTest can't be run locally because > "classnames.properties" lacks > org.apache.ignite.configuration.EncryptionConfiguration and > org.apache.ignite.configuration.PageReplacementMode. On teamcity, however, > such tests pass without any problem, because "classnames.properties" is also > generated by org.apache.ignite.tools.classgen.ClassesGenerator during > process-classes phase in maven build. > > I propose removing this file, as it is generated during the build which > means it's a pointless job keeping it up-to-date in the repository. Also > it's impossible to tell right now if it is or it is not up-to-date (without > executing ClassesGenerator). > > Kind regards, Semyon. > -- Best regards, Ivan Pavlukhin
Re: Proposal to remove explicit "GC disable" startup suggestion
Ilya, hi. +1 from me. We should not worry about weird user actions. Direct calling of GC looks unusual today. Suggestion of forced disabling manual System.gc() looks useless. 14.05.2021 15:06, Ilya Korol пишет: Hi, everyone. There is a proposal to remove suggestion that user should disable explicit GC calls in https://issues.apache.org/jira/browse/IGNITE-14720. Nowadays people usually don't use this facility directly without huge need (at least we hope so), so this suggestion doesn't bring much profit, but instead could lead to potential problems with reclaiming memory used by direct memory buffers. I'm going to fix this. Is there any objections (suggestions)?
Re: [DISCUSS] Python thin client development approach.
Igniters. Almost half of a year passed since this thread begun. We released 0.4.0, we adopted travis-ci and use it as primary source for test results but nothing changed in TC 1. We use deprecated pytest-runner (even here (https://pypi.org/project/pytest-runner) you can see deprecation notice and advice to use tox instead) in TC. This dependency is absolutely unnecessary and cause a bunch of problems when pyignite is installed as source from pypi.org local mirror We just need at least tox in linux agents on TC. 2. We build release 0.4.0 manually on my and Igor Sapego's laptops. There are no release build on TC, despite the fact that all needed scripts are included in git repo and comprehensive instruction is available. For running these scripts, we just need docker available on linux agents and some additional packages on windows agents. I, Igor and Petr Ivanov had personal talk about this few months ago but nothing changed. пт, 22 янв. 2021 г. в 16:12, Ivan Daschinsky : > > Igor, I've never talked about complete removal of TC builds. > I just suggested to add TC jobs for different python versions and use travis > heavily. > > Currently, we have done most of the tasks, except of run TC tests on > different python's versions. > > > > пт, 22 янв. 2021 г. в 15:16, Igor Sapego : >> >> Ivan, >> >> Though I generally agree with the approach you've suggested, I can see >> a problem here. Since we now have a separate repos for thin clients, for >> some features we may need to introduce changes to Ignite and python-thin >> repos in a single ticket and we should have an ability to run tests on with >> changes on both python client and server nodes. Current TC suites provide >> such ability, Travis does not. So I believe, it's too soon to abandon TC >> for thin >> clients, at least until we could solve the issue I've described. >> >> Best Regards, >> Igor >> >> >> On Fri, Dec 25, 2020 at 1:49 PM Nikolay Izhikov wrote: >> >> > Hello, Ivan. >> > >> > I’m +1 for your proposal. >> > >> > > 25 дек. 2020 г., в 13:14, Ivan Daschinsky >> > написал(а): >> > > >> > > Hi folks! >> > > >> > > Since we already have a separate repo for thin-clients [1], [2] >> > > I'd like to propose some improvements in development process/ >> > > >> > > 1. We should simplify and automate unit tests run for different versions >> > of >> > > python >> > > 2. We should add travis integration per commit and pr. Tests could be run >> > > against latest dockered image of ignite >> > > 3. There should be ability to run tests against multiple pythons on TC >> > > 4. For thin client development process, travis should be the first >> > option. >> > > TC suite should be used only to check that compatibility is not broken >> > > and when new functionality is developed (rare case). >> > > >> > > I've prepared fix [3], you can see successful builds for travis. It uses >> > > tox [5], the most common tool to run tests in multiple environments. >> > > There are few environments set up in tox.ini -- with and without docker, >> > > with or without ssl, etc. This helped a lot >> > > to setup travis CI build (you can see in commits list of PR) and simplify >> > > run tests for developers. Also docker-compose was introduced >> > > to help python thin client developers. >> > > >> > > But I need some assistance for TC: >> > > 1. There is outdated python setuptools on TC agents, so tests cannot be >> > run >> > > with updated pytest etc. >> > > 2. Multiple pythons should be installed on TC agents (via >> > > https://github.com/pyenv/pyenv), latest minor versions >> > > for 3.6, 3.7 and 3.8 >> > > 3. After that, TC job should be changed to utilize tox >> > > >> > > WDYT about this initiative? >> > > >> > > >> > > [1] -- https://issues.apache.org/jira/browse/IGNITE-13767 >> > > [2] -- https://github.com/apache/ignite-python-thin-client >> > > [3] -- https://issues.apache.org/jira/browse/IGNITE-13903 >> > > [4] -- >> > https://github.com/apache/ignite-python-thin-client/pull/1/commits >> > > [5] -- https://tox.readthedocs.io/en/latest/ >> > > >> > > -- >> > > Sincerely yours, Ivan Daschinskiy >> > >> > > > > > -- > Sincerely yours, Ivan Daschinskiy -- Sincerely yours, Ivan Daschinskiy
Re: [DISCUSS] Python thin client development approach.
Ivan, if Petr can`t help here, how can we change it out own ? May be we need some help from pmc chair or someone else ? > >> >>Igniters. Almost half of a year passed since this thread begun. We >>released 0.4.0, we adopted travis-ci and use it as primary source for >>test results but nothing changed in TC >> >>1. We use deprecated pytest-runner (even here >>( https://pypi.org/project/pytest-runner ) you can see deprecation >>notice and advice to use tox instead) in TC. >> This dependency is absolutely unnecessary and cause a bunch of >>problems when pyignite is installed as source from pypi.org local >>mirror >> We just need at least tox in linux agents on TC. >>2. We build release 0.4.0 manually on my and Igor Sapego's laptops. >>There are no release build on TC, despite the fact that all needed >>scripts are included in git repo and comprehensive instruction is >>available. >>For running these scripts, we just need docker available on linux >>agents and some additional packages on windows agents. >> >>I, Igor and Petr Ivanov had personal talk about this few months ago >>but nothing changed. >> >>пт, 22 янв. 2021 г. в 16:12, Ivan Daschinsky < ivanda...@gmail.com >: >>> >>> Igor, I've never talked about complete removal of TC builds. >>> I just suggested to add TC jobs for different python versions and use >>> travis heavily. >>> >>> Currently, we have done most of the tasks, except of run TC tests on >>> different python's versions. >>> >>> >>> >>> пт, 22 янв. 2021 г. в 15:16, Igor Sapego < isap...@apache.org >: Ivan, Though I generally agree with the approach you've suggested, I can see a problem here. Since we now have a separate repos for thin clients, for some features we may need to introduce changes to Ignite and python-thin repos in a single ticket and we should have an ability to run tests on with changes on both python client and server nodes. Current TC suites provide such ability, Travis does not. So I believe, it's too soon to abandon TC for thin clients, at least until we could solve the issue I've described. Best Regards, Igor On Fri, Dec 25, 2020 at 1:49 PM Nikolay Izhikov < nizhi...@apache.org > wrote: > Hello, Ivan. > > I’m +1 for your proposal. > > > 25 дек. 2020 г., в 13:14, Ivan Daschinsky < ivanda...@gmail.com > > написал(а): > > > > Hi folks! > > > > Since we already have a separate repo for thin-clients [1], [2] > > I'd like to propose some improvements in development process/ > > > > 1. We should simplify and automate unit tests run for different versions > of > > python > > 2. We should add travis integration per commit and pr. Tests could be run > > against latest dockered image of ignite > > 3. There should be ability to run tests against multiple pythons on TC > > 4. For thin client development process, travis should be the first > option. > > TC suite should be used only to check that compatibility is not broken > > and when new functionality is developed (rare case). > > > > I've prepared fix [3], you can see successful builds for travis. It uses > > tox [5], the most common tool to run tests in multiple environments. > > There are few environments set up in tox.ini -- with and without docker, > > with or without ssl, etc. This helped a lot > > to setup travis CI build (you can see in commits list of PR) and simplify > > run tests for developers. Also docker-compose was introduced > > to help python thin client developers. > > > > But I need some assistance for TC: > > 1. There is outdated python setuptools on TC agents, so tests cannot be > run > > with updated pytest etc. > > 2. Multiple pythons should be installed on TC agents (via > > https://github.com/pyenv/pyenv ), latest minor versions > > for 3.6, 3.7 and 3.8 > > 3. After that, TC job should be changed to utilize tox > > > > WDYT about this initiative? > > > > > > [1] -- https://issues.apache.org/jira/browse/IGNITE-13767 > > [2] -- https://github.com/apache/ignite-python-thin-client > > [3] -- https://issues.apache.org/jira/browse/IGNITE-13903 > > [4] -- > https://github.com/apache/ignite-python-thin-client/pull/1/commits > > [5] -- https://tox.readthedocs.io/en/latest/ > > > > -- > > Sincerely yours, Ivan Daschinskiy > > >>> >>> >>> >>> -- >>> Sincerely yours, Ivan Daschinskiy > > > >
