Re: Coding guidelines. Useless JavaDoc comments.
Although I agree we have some meaningless comments, making them optional can result at having no comments.Therefore I'd follow Nikolay's suggestion to have comments (perhaps with some exclusions for loggers, etc.) and make it optional for tests. -- Roman On Wednesday, August 7, 2019, 7:54:51 p.m. GMT+9, Andrey Kuznetsov wrote: +1 for making javadoc comments optional. - Empty and tautological comments are kind of garbage that reduce readability. - It's better to leave the entity undocumented, than write unexpressive/misleading comment. - Even classes may not require javadocs, e.g. simple DTOs. ср, 7 авг. 2019 г., 13:39 Denis Garus : > Thx for feedback! > > >> we have to write proper javadoc for all production classes, including > internal. > > Nikolay, I cannot agree with it. > > What should be the best comment for the next fields? > /** */ > private static final long serialVersionUID = 0L; > or > /** */ > @LoggerResource > private IgniteLogger log; > > There are more than 8000 lines of /** */ only at the ignite-core module (do > not include tests). > > Any comments will be redundant and just noise. Obvious comment learns > readers skip all comments. > > > >> identical distance/padding/margin between fields in a class - is really > cool > > Vyacheslav, but we have a blank line between fields. Why is one blank line > not enough? > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван : > > > Hi, > > > > Denis, thank you for starting this discussion! > > > > My opinion here is that having a good javadoc for every class and > > method is not feasible in the real world. I am quite curious to see a > > non-trivial project which follows it. Also, all comments and javadocs > > are prone to become misleading when code changes (human factor). In my > > experience good method and variable names and clear code organization > > often are more helpful than javadocs. > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur : > > > > > > I agree that useless comments look weird in the codebase. > > > > > > But, identical distance/padding/margin between fields in a class - is > > > really cool, and helps read the class very fast. > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov > > wrote: > > > > > > > > Hello, Denis. > > > > > > > > Thanks for starting this discussion. > > > > > > > > I think we have to write proper javadoc for all production classes, > > including internal. > > > > We should fix useless javadoc you provide. > > > > We should not accept patches without good javadocs. > > > > > > > > As for the tests, seems, we can make javadoc optional. > > > > > > > > What do you think? > > > > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > > > > Igniters! > > > > > > > > > > I think it's time to change coding guidelines in part of JavaDoc > > comments > > > > > [1]: > > > > > > > Every method, field or initializer public, private or protected > > in > > > > > > > > > > top-level, > > > > > > > inner or anonymous type should have at least minimal Javadoc > > comments > > > > > > > > > > including > > > > > > > description and description of parameters using @param, @return > > and > > > > > > > > > > @throws Javadoc tags, > > > > > > > where applicable. > > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > > > > > > > Why? > > > > > > > > > > /** */ - 15 matches. > > > > > > > > > > What can you get new from these comments? > > > > > > > > > > /** Periodic starvation check interval. */ > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * > 30; > > > > > > > > > > /** Long jvm pause detector. */ > > > > > private LongJVMPauseDetector longJVMPauseDetector; > > > > > > > > > > /** Scheduler. */ > > > > > private IgniteScheduler scheduler; > > > > > > > > > > /** Stop guard. */ > > > > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > > > > > > > /** > > > > > * @param cfg Configuration to use. > > > > > * @param utilityCachePool Utility cache pool. > > > > > * @param execSvc Executor service. > > > > > * @param sysExecSvc System executor service. > > > > > * @param stripedExecSvc Striped executor. > > > > > * @param p2pExecSvc P2P executor service. > > > > > * @param mgmtExecSvc Management executor service. > > > > > * @param igfsExecSvc IGFS executor service. > > > > > * @param dataStreamExecSvc data stream executor service. > > > > > * @param restExecSvc Reset executor service. > > > > > * @param affExecSvc Affinity executor service. > > > > > * @param idxExecSvc Indexing executor service. > > > > > * @param callbackExecSvc Callback executor service. > > > > > * @param qryExecSvc Query executor service. > > > > > * @param schemaExecSvc Schema executor service. > > > > > * @param customExecSvcs Custom named executors. > > > > > * @param errHnd Error handler to use for notification about > > startup > > > > >
Re: New Ignite PMC Member: Ilya Kasnacheev
Ilya, congratulations! -- Roman On Wednesday, August 7, 2019, 4:58:36 a.m. GMT+9, Denis Magda wrote: The Project Management Committee (PMC) for Apache Ignite has invited Ilya Kasnacheev to become a committer and we are pleased to announce that he has accepted. Ilya is one of the most active and valuable Ignite committers who is not only contributing source-code changes but also takes an active stance on Ignite adoption - he constantly helps Ignite users to design, troubleshoot and optimize their Ignite deployments. Being a PMC member enables assistance with the management and to guide the direction of the project. Please join me in congratulating Ilya with this new role! - Denis
Re: Batch write to data pages
Igniters, Will someone else look at these changes [1]? [1] https://github.com/apache/ignite/pull/6364 [2] https://issues.apache.org/jira/browse/IGNITE-11584 чт, 1 авг. 2019 г. в 18:53, Pavel Pereslegin : > > Igniters, > > Maxim Muzafarov and Anton Vinogradov reviewed my PR [1] for the task > described above [2], > > Dmitriy Govorukhin, Alexey Goncharuk, please take a look at these changes. > > Igniters, please, join the review. Your feedback is really appreciated. > > [1] https://github.com/apache/ignite/pull/6364 > [2] https://issues.apache.org/jira/browse/IGNITE-11584 > > пт, 14 июн. 2019 г. в 17:37, Maxim Muzafarov : > > > > Pavel, > > > > Thank you for your efforts! > > > > I'll take a look, shortly. > > > > On Fri, 14 Jun 2019 at 15:53, Pavel Pereslegin wrote: > > > > > > Hello Igniters! > > > > > > I'm working on implementing batch updates in PageMemory [1] to improve > > > the performance of batch operations. Details can be found in IEP-32 > > > [2]. > > > > > > As the first step, I have prepared PR [3] with the implementation of > > > batch insertion of several data rows into the free list [4]. > > > > > > Performance increased by reducing the workload on the free list - > > > after acquiring a memory page, we can write several data rows before > > > putting the page into the free list. This approach is used in data > > > preloading. Preloader cannot lock multiple cache entries at once due > > > to possible deadlock with concurrent batch updates, so it pre-creates > > > batch of data rows in the page memory, and then sequentially > > > initializes the cache entries one by one. > > > > > > Can someone review these changes? > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-7935 > > > [2] > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-32+Batch+updates+in+PageMemory > > > [3] https://github.com/apache/ignite/pull/6364 > > > [4] https://issues.apache.org/jira/browse/IGNITE-11584
Re: [EXTERNAL] Re: Replace or Put after PutAsync causes Ignite to hang
Hello!
I think we should definitely stop running futures out of striped pool,
while holding any cache logs (stripe thread counts as one).
Regards,
--
Ilya Kasnacheev
ср, 7 авг. 2019 г. в 17:20, Pavel Tupitsyn :
> Yes, this can be done purely on .NET side, which is an option that I
> consider.
> However, the root problem is on Java side, and I believe that we should fix
> the root problem.
>
> > violate some of Ignite assumptions: that we never run user code from
> certain thread pools
> We actually do run user code from Ignite thread pools:
>
> cache.getAsync(1).listen(fut ->
> System.out.println("Get operation completed [value=" + fut.get() +
> ']'));
>
> `println` here is executed on the striped pool. This is stated in the
> docs that I linked above.
>
> Users have to be aware of this and they have to be very careful with
> every future listener. IMO, this is a tricky gotcha and a bad
> usability.
>
>
> Thoughts?
>
>
> On Wed, Aug 7, 2019 at 12:22 PM Ilya Kasnacheev >
> wrote:
>
> > Hello!
> >
> > + dev@
> >
> > I think the current behavior, where .Net callbacks may be run from
> striped
> > pool, violate some of Ignite assumptions: that we never run user code
> from
> > certain thread pools (like sys-stripe) and that we try to limit options
> of
> > running user-supplied code from our internals.
> >
> > I think that future versions of .Net integration should remove the
> ability
> > of async callbacks to be called from non-user threads, even if it can
> lead
> > to performance degradation in some cases. I suggest removing this mode,
> if
> > possible, while keeping only the safe one, where internal threads are not
> > waiting upon completion of user code.
> >
> > In this case my issue IGNITE-12033 could be used to track this work.
> >
> > WDYT?
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 7 авг. 2019 г. в 01:47, Pavel Tupitsyn :
> >
> >> Sorry guys, I've completely missed this thread, and the topic is very
> >> important.
> >>
> >> First, a simple fix for the given example. Add the following on the
> first
> >> line of Main:
> >> SynchronizationContext.SetSynchronizationContext(new
> >> ThreadPoolSynchronizationContext());
> >>
> >> And put the ThreadPoolSynchronizationContext class somewhere:
> >> class ThreadPoolSynchronizationContext : SynchronizationContext
> >> {
> >> // No-op.
> >> }
> >>
> >>
> >> Now, detailed explanation. The problem exists forever in Ignite and is
> >> mentioned in the docs briefly [1].
> >> Also mentioned in .NET docs (I've updated them a bit) [2].
> >>
> >> Breakdown:
> >> * Ignite (Java side) runs async callbacks (continuations) on system
> >> threads, and those threads have limitations (you should not call Ignite
> >> APIs from them in general)
> >> * Ignite.NET wraps async operations into native .NET Tasks
> >> * Usually `await ...` call in .NET will continue execution on the
> >> original Thread (simply put, actually it is more complex), so Ignite
> system
> >> thread issue is avoided
> >> * However, Console applications have no `SynchronizationContext`, so the
> >> continuation can't be dispatched to original thread, and is executed on
> >> current (Ignite) thread
> >> * Setting custom SynchronizationContext fixes the issue: all async
> >> continuations will be dispatched to .NET thread pool and never executed
> on
> >> Ignite threads
> >>
> >> However, dispatching callbacks to a different thread causes performance
> >> hit, and Ignite favors performance over usability right now.
> >> So it is up to the user to configure desired behavior.
> >>
> >> Let me know if you need more details.
> >>
> >> Thanks
> >>
> >> [1] https://apacheignite.readme.io/docs/async-support
> >> [2] https://apacheignite-net.readme.io/docs/asynchronous-support
> >>
> >> On Thu, Aug 1, 2019 at 3:41 PM Ilya Kasnacheev <
> ilya.kasnach...@gmail.com>
> >> wrote:
> >>
> >>> Hello!
> >>>
> >>> I have filed a ticket about this issue so it won't get lost.
> >>> https://issues.apache.org/jira/browse/IGNITE-12033
> >>>
> >>> Regards,
> >>> --
> >>> Ilya Kasnacheev
> >>>
> >>>
> >>> чт, 2 мая 2019 г. в 10:53, Barney Pippin <
> james.pri...@uk.bnpparibas.com
> >>> >:
> >>>
> Thanks for the response Ilya. Did you get a chance to look at this
> Pavel?
> Thanks.
>
>
>
> --
> Sent from: http://apache-ignite-users.70518.x6.nabble.com/
>
> >>>
>
Re: [EXTERNAL] Re: Replace or Put after PutAsync causes Ignite to hang
Yes, this can be done purely on .NET side, which is an option that I
consider.
However, the root problem is on Java side, and I believe that we should fix
the root problem.
> violate some of Ignite assumptions: that we never run user code from
certain thread pools
We actually do run user code from Ignite thread pools:
cache.getAsync(1).listen(fut ->
System.out.println("Get operation completed [value=" + fut.get() + ']'));
`println` here is executed on the striped pool. This is stated in the
docs that I linked above.
Users have to be aware of this and they have to be very careful with
every future listener. IMO, this is a tricky gotcha and a bad
usability.
Thoughts?
On Wed, Aug 7, 2019 at 12:22 PM Ilya Kasnacheev
wrote:
> Hello!
>
> + dev@
>
> I think the current behavior, where .Net callbacks may be run from striped
> pool, violate some of Ignite assumptions: that we never run user code from
> certain thread pools (like sys-stripe) and that we try to limit options of
> running user-supplied code from our internals.
>
> I think that future versions of .Net integration should remove the ability
> of async callbacks to be called from non-user threads, even if it can lead
> to performance degradation in some cases. I suggest removing this mode, if
> possible, while keeping only the safe one, where internal threads are not
> waiting upon completion of user code.
>
> In this case my issue IGNITE-12033 could be used to track this work.
>
> WDYT?
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 7 авг. 2019 г. в 01:47, Pavel Tupitsyn :
>
>> Sorry guys, I've completely missed this thread, and the topic is very
>> important.
>>
>> First, a simple fix for the given example. Add the following on the first
>> line of Main:
>> SynchronizationContext.SetSynchronizationContext(new
>> ThreadPoolSynchronizationContext());
>>
>> And put the ThreadPoolSynchronizationContext class somewhere:
>> class ThreadPoolSynchronizationContext : SynchronizationContext
>> {
>> // No-op.
>> }
>>
>>
>> Now, detailed explanation. The problem exists forever in Ignite and is
>> mentioned in the docs briefly [1].
>> Also mentioned in .NET docs (I've updated them a bit) [2].
>>
>> Breakdown:
>> * Ignite (Java side) runs async callbacks (continuations) on system
>> threads, and those threads have limitations (you should not call Ignite
>> APIs from them in general)
>> * Ignite.NET wraps async operations into native .NET Tasks
>> * Usually `await ...` call in .NET will continue execution on the
>> original Thread (simply put, actually it is more complex), so Ignite system
>> thread issue is avoided
>> * However, Console applications have no `SynchronizationContext`, so the
>> continuation can't be dispatched to original thread, and is executed on
>> current (Ignite) thread
>> * Setting custom SynchronizationContext fixes the issue: all async
>> continuations will be dispatched to .NET thread pool and never executed on
>> Ignite threads
>>
>> However, dispatching callbacks to a different thread causes performance
>> hit, and Ignite favors performance over usability right now.
>> So it is up to the user to configure desired behavior.
>>
>> Let me know if you need more details.
>>
>> Thanks
>>
>> [1] https://apacheignite.readme.io/docs/async-support
>> [2] https://apacheignite-net.readme.io/docs/asynchronous-support
>>
>> On Thu, Aug 1, 2019 at 3:41 PM Ilya Kasnacheev
>> wrote:
>>
>>> Hello!
>>>
>>> I have filed a ticket about this issue so it won't get lost.
>>> https://issues.apache.org/jira/browse/IGNITE-12033
>>>
>>> Regards,
>>> --
>>> Ilya Kasnacheev
>>>
>>>
>>> чт, 2 мая 2019 г. в 10:53, Barney Pippin >> >:
>>>
Thanks for the response Ilya. Did you get a chance to look at this
Pavel?
Thanks.
--
Sent from: http://apache-ignite-users.70518.x6.nabble.com/
>>>
Re: [DISCUSSION] Channel communication between nodes
Maxim, thank you! Great job! PR looks good to me. Who else want to take a look? В Ср, 07/08/2019 в 17:01 +0300, Maxim Muzafarov пишет: > Igniters, > > I think the PR [2] of the issue [1] is ready to be merged. > Thanks to Nikolay Izhikov for the great review! > The IEP-28 confluence page [4] is also updated with new API details. > > Will anyone take an additional review of this improvement? > > Some limitations and drawbacks of the current implementation which I > will fix in additional issues: > - CommunicationSpi connections\opened ports are completely reused with > the current value of port range 100. It is better to have a separate > configuration of port ranges for such type of connections (probably > not); > - Files send between cluster nodes by chunks of size defined in > DFLT_CHUNK_SIZE_BYTES constant. Currently, it's not configurable > parameter it is better to move it to configuration; > - Transmission bandwidth limitations are implemented but excluded from > current PR to simplify the review. Will be added in a separate ticket; > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > [2] https://github.com/apache/ignite/pull/5619 > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > [4] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-Filetransferbetweennodes > > On Thu, 30 May 2019 at 20:01, Maxim Muzafarov wrote: > > > > Folks, > > > > > > The PR [2] has been updated and comments [1] have been addressed. > > If you're interested in this feature, please join the review [3] and > > discussion. > > > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > > [2] https://github.com/apache/ignite/pull/5619 > > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > > > > On Tue, 21 May 2019 at 16:07, Maxim Muzafarov wrote: > > > > > > Alexey, > > > > > > Thank you for your feedback. I'll take a bit of time to re-think an > > > API and its usage according to your suggestions and back to you soon > > > with the detailed response. > > > > > > But in general, I'd like to focus your attention on the following idea > > > of this PR, which does not fully correspond to the title of this topic > > > (perhaps I have to change the topic title, but I can't do it anymore). > > > This change is not about providing the raw socket channel to the > > > Supplier for transferring binary data through it, but creating an > > > internal service for sending\receiving files between nodes which can > > > be reused by other Apache Ignite components. I think it's better to > > > move this file transfer machinery outside the supply-demand > > > communication protocol and make it an independent abstraction > > > (service) for sending\receiving any Ignites internal files. > > > From the IEP-28 perspective, the hierarchy of such objects can be: > > > Channel > GridIoManager > Send file Service > Preloader (backup > > > partition and send it to remote the Demander node). > > > > > > Please, correct me if I'm missing something or have some > > > misunderstanding of Ignite internals. > > > > > > On Mon, 20 May 2019 at 11:56, Alexey Goncharuk > > > wrote: > > > > > > > > Maxim, > > > > > > > > I left response in the ticket. > > > > > > > > пт, 17 мая 2019 г. в 12:04, Maxim Muzafarov : > > > > > > > > > Igniters, > > > > > > > > > > > > > > > I've implemented the file transfer machinery between grid nodes over > > > > > Communication SPI covered by JIRA [1] and as the first part of IEP-28 > > > > > [3]. Please, consider my PR [2] to be reviewed and included in the > > > > > Apache Ignite project source code. > > > > > > > > > > These changes will allow developer (and the user in future) uploading > > > > > arbitrary files from one cluster node to another. It is a mandatory > > > > > feature for the new rebalance by partition files procedure (you can > > > > > find all the details of it on IEP-28 [3]) but also can be used, for > > > > > example, as a new way of resource deployment (e.g. on > > > > > UriDeploymentSPI). > > > > > > > > > > The starting point of PR reviewing and examples of using the file > > > > > transfer can be -- > > > > > IgniteFileTransmitProcessorSelfTest#testFileHandlerBase. Any feedback > > > > > is really appreciated. Please, don't be silent. > > > > > > > > > > > > > > > PR [2] contains two major changes (components): > > > > > > > > > > 1) Adding channel support for the Communication SPI, which allows > > > > > from now: > > > > > - establishing channel connections to the remote node to an arbitrary > > > > > topic (GridTopic) with predefined channel processing policy; > > > > > - listening to incoming channel creation events and registering > > > > > connection handlers on the particular node; > > > > > - an arbitrary set of channel attributes configured on connection > > > > > handshake (exchanging the channel attributes ); > > > > > The main API changes in the
Re: [DISCUSSION] Channel communication between nodes
Igniters, I think the PR [2] of the issue [1] is ready to be merged. Thanks to Nikolay Izhikov for the great review! The IEP-28 confluence page [4] is also updated with new API details. Will anyone take an additional review of this improvement? Some limitations and drawbacks of the current implementation which I will fix in additional issues: - CommunicationSpi connections\opened ports are completely reused with the current value of port range 100. It is better to have a separate configuration of port ranges for such type of connections (probably not); - Files send between cluster nodes by chunks of size defined in DFLT_CHUNK_SIZE_BYTES constant. Currently, it's not configurable parameter it is better to move it to configuration; - Transmission bandwidth limitations are implemented but excluded from current PR to simplify the review. Will be added in a separate ticket; [1] https://issues.apache.org/jira/browse/IGNITE-10619 [2] https://github.com/apache/ignite/pull/5619 [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 [4] https://cwiki.apache.org/confluence/display/IGNITE/IEP-28%3A+Cluster+peer-2-peer+balancing#IEP-28:Clusterpeer-2-peerbalancing-Filetransferbetweennodes On Thu, 30 May 2019 at 20:01, Maxim Muzafarov wrote: > > Folks, > > > The PR [2] has been updated and comments [1] have been addressed. > If you're interested in this feature, please join the review [3] and > discussion. > > [1] https://issues.apache.org/jira/browse/IGNITE-10619 > [2] https://github.com/apache/ignite/pull/5619 > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1035 > > On Tue, 21 May 2019 at 16:07, Maxim Muzafarov wrote: > > > > Alexey, > > > > Thank you for your feedback. I'll take a bit of time to re-think an > > API and its usage according to your suggestions and back to you soon > > with the detailed response. > > > > But in general, I'd like to focus your attention on the following idea > > of this PR, which does not fully correspond to the title of this topic > > (perhaps I have to change the topic title, but I can't do it anymore). > > This change is not about providing the raw socket channel to the > > Supplier for transferring binary data through it, but creating an > > internal service for sending\receiving files between nodes which can > > be reused by other Apache Ignite components. I think it's better to > > move this file transfer machinery outside the supply-demand > > communication protocol and make it an independent abstraction > > (service) for sending\receiving any Ignites internal files. > > From the IEP-28 perspective, the hierarchy of such objects can be: > > Channel > GridIoManager > Send file Service > Preloader (backup > > partition and send it to remote the Demander node). > > > > Please, correct me if I'm missing something or have some > > misunderstanding of Ignite internals. > > > > On Mon, 20 May 2019 at 11:56, Alexey Goncharuk > > wrote: > > > > > > Maxim, > > > > > > I left response in the ticket. > > > > > > пт, 17 мая 2019 г. в 12:04, Maxim Muzafarov : > > > > > > > Igniters, > > > > > > > > > > > > I've implemented the file transfer machinery between grid nodes over > > > > Communication SPI covered by JIRA [1] and as the first part of IEP-28 > > > > [3]. Please, consider my PR [2] to be reviewed and included in the > > > > Apache Ignite project source code. > > > > > > > > These changes will allow developer (and the user in future) uploading > > > > arbitrary files from one cluster node to another. It is a mandatory > > > > feature for the new rebalance by partition files procedure (you can > > > > find all the details of it on IEP-28 [3]) but also can be used, for > > > > example, as a new way of resource deployment (e.g. on > > > > UriDeploymentSPI). > > > > > > > > The starting point of PR reviewing and examples of using the file > > > > transfer can be -- > > > > IgniteFileTransmitProcessorSelfTest#testFileHandlerBase. Any feedback > > > > is really appreciated. Please, don't be silent. > > > > > > > > > > > > PR [2] contains two major changes (components): > > > > > > > > 1) Adding channel support for the Communication SPI, which allows from > > > > now: > > > > - establishing channel connections to the remote node to an arbitrary > > > > topic (GridTopic) with predefined channel processing policy; > > > > - listening to incoming channel creation events and registering > > > > connection handlers on the particular node; > > > > - an arbitrary set of channel attributes configured on connection > > > > handshake (exchanging the channel attributes ); > > > > The main API changes in the communication component can be found here > > > > [5]. > > > > > > > > 2) Creating a new FileTransmitProcessor, which provide an API for > > > > sending and receiving files. It supports: > > > > - using different approaches of incoming data handling – buffered and > > > > direct (zero-copy approach of FileChannel#transferTo); > > > > - transferring data by chunks of
Re: New Ignite PMC Member: Ilya Kasnacheev
Glad to hear! Good luck with new solutions! ср, 7 авг. 2019 г. в 12:05, Ilya Kasnacheev : > Hello! > > I hope to uphold your trust. > > Thanks everyone for your support! > > Regards, > -- > Ilya Kasnacheev > > > ср, 7 авг. 2019 г. в 09:36, Vyacheslav Daradur : > > > Ilya, congratulations! > > > > On Wed, Aug 7, 2019 at 8:41 AM Павлухин Иван > wrote: > > > > > > Ilya, my congratulations! > > > > > > вт, 6 авг. 2019 г. в 23:12, Dmitriy Pavlov : > > > > > > > > Ilya, congratulations! > > > > > > > > вт, 6 авг. 2019 г. в 22:58, Denis Magda : > > > > > > > > > The Project Management Committee (PMC) for Apache Ignite > > > > > has invited Ilya Kasnacheev to become a committer and we are > pleased > > > > > to announce that he has accepted. > > > > > > > > > > Ilya is one of the most active and valuable Ignite committers who > is > > not > > > > > only contributing source-code changes but also takes an active > > stance on > > > > > Ignite adoption - he constantly helps Ignite users to design, > > troubleshoot > > > > > and optimize their Ignite deployments. > > > > > > > > > > Being a PMC member enables assistance with the management and to > > guide the > > > > > direction of the project. > > > > > > > > > > Please join me in congratulating Ilya with this new role! > > > > > > > > > > > > > > > - > > > > > Denis > > > > > > > > > > > > > > > > > -- > > > Best regards, > > > Ivan Pavlukhin > > > > > > > > -- > > Best Regards, Vyacheslav D. > > >
[jira] [Created] (IGNITE-12049) Allow custom authenticators to use SSL certificates
Ryabov Dmitrii created IGNITE-12049: --- Summary: Allow custom authenticators to use SSL certificates Key: IGNITE-12049 URL: https://issues.apache.org/jira/browse/IGNITE-12049 Project: Ignite Issue Type: Improvement Reporter: Ryabov Dmitrii Add SSL certificates to AuthenticationContext, so, authenticators can make additional checks based on SSL certificates. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Created] (IGNITE-12048) Bugs & tests fixes
Dmitriy Govorukhin created IGNITE-12048:
---
Summary: Bugs & tests fixes
Key: IGNITE-12048
URL: https://issues.apache.org/jira/browse/IGNITE-12048
Project: Ignite
Issue Type: Bug
Reporter: Dmitriy Govorukhin
Page replacement can reload invalid page during checkpoint
There is a race between {{writeCheckpointPages}} and page replacement process:
* Checkpointer thread begins a checkpoint
* Checkpointer thread calls {{getPageForCheckpoint()}}, which will copy page
content *and clear dirty flag*
* Page replacement tries to find a page for replacement and chooses this page,
the page is thrown away
* Before the page is written back to the store, the page is acquired again.
As a result, an older copy of the page is brought back to memory, which causes
all kinds of corruption exceptions and assertions.
-
checkpointReadLock() may hang during node stop
I got this hang during one of PDS (Indexing) runs (thread-dump is attached).
The following code hang:
{code:java}
checkpointer.wakeupForCheckpoint(0, "too many dirty pages").cpBeginFut
.getUninterruptibly();
{code}
It looks like {{wakeupForCheckpoint}} can be called after the checkpointer is
stopped and {{cpBeginFut}} will be never completed.
-
Fixed
ZookeeperDiscoveryCommunicationFailureTest.testCommunicationFailureResolve_CachesInfo1
Fixed *.testFailAfterStart
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[MTCGA]: new failures in builds [4453375] needs to be handled
Hi Igniters, I've detected some new issue on TeamCity to be handled. You are more than welcomed to help. *New Critical Failure in master Cache (Restarts) 1 https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CacheRestarts1?branch=%3Cdefault%3E No changes in the build - Here's a reminder of what contributors were agreed to do https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute - Should you have any questions please contact dev@ignite.apache.org Best Regards, Apache Ignite TeamCity Bot https://github.com/apache/ignite-teamcity-bot Notification generated at 14:03:36 07-08-2019
Re: Coding guidelines. Useless JavaDoc comments.
+1 for making javadoc comments optional. - Empty and tautological comments are kind of garbage that reduce readability. - It's better to leave the entity undocumented, than write unexpressive/misleading comment. - Even classes may not require javadocs, e.g. simple DTOs. ср, 7 авг. 2019 г., 13:39 Denis Garus : > Thx for feedback! > > >> we have to write proper javadoc for all production classes, including > internal. > > Nikolay, I cannot agree with it. > > What should be the best comment for the next fields? > /** */ > private static final long serialVersionUID = 0L; > or > /** */ > @LoggerResource > private IgniteLogger log; > > There are more than 8000 lines of /** */ only at the ignite-core module (do > not include tests). > > Any comments will be redundant and just noise. Obvious comment learns > readers skip all comments. > > > >> identical distance/padding/margin between fields in a class - is really > cool > > Vyacheslav, but we have a blank line between fields. Why is one blank line > not enough? > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван : > > > Hi, > > > > Denis, thank you for starting this discussion! > > > > My opinion here is that having a good javadoc for every class and > > method is not feasible in the real world. I am quite curious to see a > > non-trivial project which follows it. Also, all comments and javadocs > > are prone to become misleading when code changes (human factor). In my > > experience good method and variable names and clear code organization > > often are more helpful than javadocs. > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur : > > > > > > I agree that useless comments look weird in the codebase. > > > > > > But, identical distance/padding/margin between fields in a class - is > > > really cool, and helps read the class very fast. > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov > > wrote: > > > > > > > > Hello, Denis. > > > > > > > > Thanks for starting this discussion. > > > > > > > > I think we have to write proper javadoc for all production classes, > > including internal. > > > > We should fix useless javadoc you provide. > > > > We should not accept patches without good javadocs. > > > > > > > > As for the tests, seems, we can make javadoc optional. > > > > > > > > What do you think? > > > > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > > > > Igniters! > > > > > > > > > > I think it's time to change coding guidelines in part of JavaDoc > > comments > > > > > [1]: > > > > > > > Every method, field or initializer public, private or protected > > in > > > > > > > > > > top-level, > > > > > > > inner or anonymous type should have at least minimal Javadoc > > comments > > > > > > > > > > including > > > > > > > description and description of parameters using @param, @return > > and > > > > > > > > > > @throws Javadoc tags, > > > > > > > where applicable. > > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > > > > > > > Why? > > > > > > > > > > /** */ - 15 matches. > > > > > > > > > > What can you get new from these comments? > > > > > > > > > > /** Periodic starvation check interval. */ > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * > 30; > > > > > > > > > > /** Long jvm pause detector. */ > > > > > private LongJVMPauseDetector longJVMPauseDetector; > > > > > > > > > > /** Scheduler. */ > > > > > private IgniteScheduler scheduler; > > > > > > > > > > /** Stop guard. */ > > > > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > > > > > > > /** > > > > > * @param cfg Configuration to use. > > > > > * @param utilityCachePool Utility cache pool. > > > > > * @param execSvc Executor service. > > > > > * @param sysExecSvc System executor service. > > > > > * @param stripedExecSvc Striped executor. > > > > > * @param p2pExecSvc P2P executor service. > > > > > * @param mgmtExecSvc Management executor service. > > > > > * @param igfsExecSvc IGFS executor service. > > > > > * @param dataStreamExecSvc data stream executor service. > > > > > * @param restExecSvc Reset executor service. > > > > > * @param affExecSvc Affinity executor service. > > > > > * @param idxExecSvc Indexing executor service. > > > > > * @param callbackExecSvc Callback executor service. > > > > > * @param qryExecSvc Query executor service. > > > > > * @param schemaExecSvc Schema executor service. > > > > > * @param customExecSvcs Custom named executors. > > > > > * @param errHnd Error handler to use for notification about > > startup > > > > > problems. > > > > > * @param workerRegistry Worker registry. > > > > > * @param hnd Default uncaught exception handler used by thread > > pools. > > > > > * @throws IgniteCheckedException Thrown in case of any errors. > > > > > */ > > > > > public void start( > > > > > final IgniteConfiguration cfg, > > > > >
Re: Coding guidelines. Useless JavaDoc comments.
Thx for feedback! >> we have to write proper javadoc for all production classes, including internal. Nikolay, I cannot agree with it. What should be the best comment for the next fields? /** */ private static final long serialVersionUID = 0L; or /** */ @LoggerResource private IgniteLogger log; There are more than 8000 lines of /** */ only at the ignite-core module (do not include tests). Any comments will be redundant and just noise. Obvious comment learns readers skip all comments. >> identical distance/padding/margin between fields in a class - is really cool Vyacheslav, but we have a blank line between fields. Why is one blank line not enough? ср, 7 авг. 2019 г. в 12:58, Павлухин Иван : > Hi, > > Denis, thank you for starting this discussion! > > My opinion here is that having a good javadoc for every class and > method is not feasible in the real world. I am quite curious to see a > non-trivial project which follows it. Also, all comments and javadocs > are prone to become misleading when code changes (human factor). In my > experience good method and variable names and clear code organization > often are more helpful than javadocs. > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur : > > > > I agree that useless comments look weird in the codebase. > > > > But, identical distance/padding/margin between fields in a class - is > > really cool, and helps read the class very fast. > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov > wrote: > > > > > > Hello, Denis. > > > > > > Thanks for starting this discussion. > > > > > > I think we have to write proper javadoc for all production classes, > including internal. > > > We should fix useless javadoc you provide. > > > We should not accept patches without good javadocs. > > > > > > As for the tests, seems, we can make javadoc optional. > > > > > > What do you think? > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > > > Igniters! > > > > > > > > I think it's time to change coding guidelines in part of JavaDoc > comments > > > > [1]: > > > > > > Every method, field or initializer public, private or protected > in > > > > > > > > top-level, > > > > > > inner or anonymous type should have at least minimal Javadoc > comments > > > > > > > > including > > > > > > description and description of parameters using @param, @return > and > > > > > > > > @throws Javadoc tags, > > > > > > where applicable. > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > > > > > Why? > > > > > > > > /** */ - 15 matches. > > > > > > > > What can you get new from these comments? > > > > > > > > /** Periodic starvation check interval. */ > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; > > > > > > > > /** Long jvm pause detector. */ > > > > private LongJVMPauseDetector longJVMPauseDetector; > > > > > > > > /** Scheduler. */ > > > > private IgniteScheduler scheduler; > > > > > > > > /** Stop guard. */ > > > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > > > > > /** > > > > * @param cfg Configuration to use. > > > > * @param utilityCachePool Utility cache pool. > > > > * @param execSvc Executor service. > > > > * @param sysExecSvc System executor service. > > > > * @param stripedExecSvc Striped executor. > > > > * @param p2pExecSvc P2P executor service. > > > > * @param mgmtExecSvc Management executor service. > > > > * @param igfsExecSvc IGFS executor service. > > > > * @param dataStreamExecSvc data stream executor service. > > > > * @param restExecSvc Reset executor service. > > > > * @param affExecSvc Affinity executor service. > > > > * @param idxExecSvc Indexing executor service. > > > > * @param callbackExecSvc Callback executor service. > > > > * @param qryExecSvc Query executor service. > > > > * @param schemaExecSvc Schema executor service. > > > > * @param customExecSvcs Custom named executors. > > > > * @param errHnd Error handler to use for notification about > startup > > > > problems. > > > > * @param workerRegistry Worker registry. > > > > * @param hnd Default uncaught exception handler used by thread > pools. > > > > * @throws IgniteCheckedException Thrown in case of any errors. > > > > */ > > > > public void start( > > > > final IgniteConfiguration cfg, > > > > ExecutorService utilityCachePool, > > > > final ExecutorService execSvc, > > > > final ExecutorService svcExecSvc, > > > > final ExecutorService sysExecSvc, > > > > final StripedExecutor stripedExecSvc, > > > > ExecutorService p2pExecSvc, > > > > ExecutorService mgmtExecSvc, > > > > ExecutorService igfsExecSvc, > > > > StripedExecutor dataStreamExecSvc, > > > > ExecutorService restExecSvc, > > > > ExecutorService affExecSvc, > > > > @Nullable ExecutorService idxExecSvc, > > > >
Re: Coding guidelines. Useless JavaDoc comments.
Hi, Denis, thank you for starting this discussion! My opinion here is that having a good javadoc for every class and method is not feasible in the real world. I am quite curious to see a non-trivial project which follows it. Also, all comments and javadocs are prone to become misleading when code changes (human factor). In my experience good method and variable names and clear code organization often are more helpful than javadocs. ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur : > > I agree that useless comments look weird in the codebase. > > But, identical distance/padding/margin between fields in a class - is > really cool, and helps read the class very fast. > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov wrote: > > > > Hello, Denis. > > > > Thanks for starting this discussion. > > > > I think we have to write proper javadoc for all production classes, > > including internal. > > We should fix useless javadoc you provide. > > We should not accept patches without good javadocs. > > > > As for the tests, seems, we can make javadoc optional. > > > > What do you think? > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > > Igniters! > > > > > > I think it's time to change coding guidelines in part of JavaDoc comments > > > [1]: > > > > > Every method, field or initializer public, private or protected in > > > > > > top-level, > > > > > inner or anonymous type should have at least minimal Javadoc comments > > > > > > including > > > > > description and description of parameters using @param, @return and > > > > > > @throws Javadoc tags, > > > > > where applicable. > > > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > > > Why? > > > > > > /** */ - 15 matches. > > > > > > What can you get new from these comments? > > > > > > /** Periodic starvation check interval. */ > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; > > > > > > /** Long jvm pause detector. */ > > > private LongJVMPauseDetector longJVMPauseDetector; > > > > > > /** Scheduler. */ > > > private IgniteScheduler scheduler; > > > > > > /** Stop guard. */ > > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > > > /** > > > * @param cfg Configuration to use. > > > * @param utilityCachePool Utility cache pool. > > > * @param execSvc Executor service. > > > * @param sysExecSvc System executor service. > > > * @param stripedExecSvc Striped executor. > > > * @param p2pExecSvc P2P executor service. > > > * @param mgmtExecSvc Management executor service. > > > * @param igfsExecSvc IGFS executor service. > > > * @param dataStreamExecSvc data stream executor service. > > > * @param restExecSvc Reset executor service. > > > * @param affExecSvc Affinity executor service. > > > * @param idxExecSvc Indexing executor service. > > > * @param callbackExecSvc Callback executor service. > > > * @param qryExecSvc Query executor service. > > > * @param schemaExecSvc Schema executor service. > > > * @param customExecSvcs Custom named executors. > > > * @param errHnd Error handler to use for notification about startup > > > problems. > > > * @param workerRegistry Worker registry. > > > * @param hnd Default uncaught exception handler used by thread pools. > > > * @throws IgniteCheckedException Thrown in case of any errors. > > > */ > > > public void start( > > > final IgniteConfiguration cfg, > > > ExecutorService utilityCachePool, > > > final ExecutorService execSvc, > > > final ExecutorService svcExecSvc, > > > final ExecutorService sysExecSvc, > > > final StripedExecutor stripedExecSvc, > > > ExecutorService p2pExecSvc, > > > ExecutorService mgmtExecSvc, > > > ExecutorService igfsExecSvc, > > > StripedExecutor dataStreamExecSvc, > > > ExecutorService restExecSvc, > > > ExecutorService affExecSvc, > > > @Nullable ExecutorService idxExecSvc, > > > IgniteStripedThreadPoolExecutor callbackExecSvc, > > > ExecutorService qryExecSvc, > > > ExecutorService schemaExecSvc, > > > @Nullable final Map > > > customExecSvcs, > > > GridAbsClosure errHnd, > > > WorkersRegistry workerRegistry, > > > Thread.UncaughtExceptionHandler hnd, > > > TimeBag startTimer > > > ) > > > > > > These comments look ugly and useless, and that is the main class of core. > > > Why do they need us? > > > Let us change the coding guidelines in part of JavaDoc comments: > > > Every method public API should have at least minimal Javadoc comments, > > > including description and description of parameters using @param, @return, > > > and @throws Javadoc tags, > > > where applicable. > > > For internal API, JavaDoc comments should be optional. It's up to a > > > contributor or reviewer. > > > > > > What are you think? > > > > > > 1. >
Re: Coding guidelines. Useless JavaDoc comments.
I agree that useless comments look weird in the codebase. But, identical distance/padding/margin between fields in a class - is really cool, and helps read the class very fast. On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov wrote: > > Hello, Denis. > > Thanks for starting this discussion. > > I think we have to write proper javadoc for all production classes, including > internal. > We should fix useless javadoc you provide. > We should not accept patches without good javadocs. > > As for the tests, seems, we can make javadoc optional. > > What do you think? > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > > Igniters! > > > > I think it's time to change coding guidelines in part of JavaDoc comments > > [1]: > > > > Every method, field or initializer public, private or protected in > > > > top-level, > > > > inner or anonymous type should have at least minimal Javadoc comments > > > > including > > > > description and description of parameters using @param, @return and > > > > @throws Javadoc tags, > > > > where applicable. > > > > Let's look at JavaDoc comments in the IgniteKernal class: > > > > Why? > > > > /** */ - 15 matches. > > > > What can you get new from these comments? > > > > /** Periodic starvation check interval. */ > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; > > > > /** Long jvm pause detector. */ > > private LongJVMPauseDetector longJVMPauseDetector; > > > > /** Scheduler. */ > > private IgniteScheduler scheduler; > > > > /** Stop guard. */ > > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > > > /** > > * @param cfg Configuration to use. > > * @param utilityCachePool Utility cache pool. > > * @param execSvc Executor service. > > * @param sysExecSvc System executor service. > > * @param stripedExecSvc Striped executor. > > * @param p2pExecSvc P2P executor service. > > * @param mgmtExecSvc Management executor service. > > * @param igfsExecSvc IGFS executor service. > > * @param dataStreamExecSvc data stream executor service. > > * @param restExecSvc Reset executor service. > > * @param affExecSvc Affinity executor service. > > * @param idxExecSvc Indexing executor service. > > * @param callbackExecSvc Callback executor service. > > * @param qryExecSvc Query executor service. > > * @param schemaExecSvc Schema executor service. > > * @param customExecSvcs Custom named executors. > > * @param errHnd Error handler to use for notification about startup > > problems. > > * @param workerRegistry Worker registry. > > * @param hnd Default uncaught exception handler used by thread pools. > > * @throws IgniteCheckedException Thrown in case of any errors. > > */ > > public void start( > > final IgniteConfiguration cfg, > > ExecutorService utilityCachePool, > > final ExecutorService execSvc, > > final ExecutorService svcExecSvc, > > final ExecutorService sysExecSvc, > > final StripedExecutor stripedExecSvc, > > ExecutorService p2pExecSvc, > > ExecutorService mgmtExecSvc, > > ExecutorService igfsExecSvc, > > StripedExecutor dataStreamExecSvc, > > ExecutorService restExecSvc, > > ExecutorService affExecSvc, > > @Nullable ExecutorService idxExecSvc, > > IgniteStripedThreadPoolExecutor callbackExecSvc, > > ExecutorService qryExecSvc, > > ExecutorService schemaExecSvc, > > @Nullable final Map > > customExecSvcs, > > GridAbsClosure errHnd, > > WorkersRegistry workerRegistry, > > Thread.UncaughtExceptionHandler hnd, > > TimeBag startTimer > > ) > > > > These comments look ugly and useless, and that is the main class of core. > > Why do they need us? > > Let us change the coding guidelines in part of JavaDoc comments: > > Every method public API should have at least minimal Javadoc comments, > > including description and description of parameters using @param, @return, > > and @throws Javadoc tags, > > where applicable. > > For internal API, JavaDoc comments should be optional. It's up to a > > contributor or reviewer. > > > > What are you think? > > > > 1. > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments -- Best Regards, Vyacheslav D.
Re: Coding guidelines. Useless JavaDoc comments.
Hello, Denis. Thanks for starting this discussion. I think we have to write proper javadoc for all production classes, including internal. We should fix useless javadoc you provide. We should not accept patches without good javadocs. As for the tests, seems, we can make javadoc optional. What do you think? В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет: > Igniters! > > I think it's time to change coding guidelines in part of JavaDoc comments > [1]: > > > Every method, field or initializer public, private or protected in > > top-level, > > > inner or anonymous type should have at least minimal Javadoc comments > > including > > > description and description of parameters using @param, @return and > > @throws Javadoc tags, > > > where applicable. > > Let's look at JavaDoc comments in the IgniteKernal class: > > Why? > > /** */ - 15 matches. > > What can you get new from these comments? > > /** Periodic starvation check interval. */ > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; > > /** Long jvm pause detector. */ > private LongJVMPauseDetector longJVMPauseDetector; > > /** Scheduler. */ > private IgniteScheduler scheduler; > > /** Stop guard. */ > private final AtomicBoolean stopGuard = new AtomicBoolean(); > > /** > * @param cfg Configuration to use. > * @param utilityCachePool Utility cache pool. > * @param execSvc Executor service. > * @param sysExecSvc System executor service. > * @param stripedExecSvc Striped executor. > * @param p2pExecSvc P2P executor service. > * @param mgmtExecSvc Management executor service. > * @param igfsExecSvc IGFS executor service. > * @param dataStreamExecSvc data stream executor service. > * @param restExecSvc Reset executor service. > * @param affExecSvc Affinity executor service. > * @param idxExecSvc Indexing executor service. > * @param callbackExecSvc Callback executor service. > * @param qryExecSvc Query executor service. > * @param schemaExecSvc Schema executor service. > * @param customExecSvcs Custom named executors. > * @param errHnd Error handler to use for notification about startup > problems. > * @param workerRegistry Worker registry. > * @param hnd Default uncaught exception handler used by thread pools. > * @throws IgniteCheckedException Thrown in case of any errors. > */ > public void start( > final IgniteConfiguration cfg, > ExecutorService utilityCachePool, > final ExecutorService execSvc, > final ExecutorService svcExecSvc, > final ExecutorService sysExecSvc, > final StripedExecutor stripedExecSvc, > ExecutorService p2pExecSvc, > ExecutorService mgmtExecSvc, > ExecutorService igfsExecSvc, > StripedExecutor dataStreamExecSvc, > ExecutorService restExecSvc, > ExecutorService affExecSvc, > @Nullable ExecutorService idxExecSvc, > IgniteStripedThreadPoolExecutor callbackExecSvc, > ExecutorService qryExecSvc, > ExecutorService schemaExecSvc, > @Nullable final Map > customExecSvcs, > GridAbsClosure errHnd, > WorkersRegistry workerRegistry, > Thread.UncaughtExceptionHandler hnd, > TimeBag startTimer > ) > > These comments look ugly and useless, and that is the main class of core. > Why do they need us? > Let us change the coding guidelines in part of JavaDoc comments: > Every method public API should have at least minimal Javadoc comments, > including description and description of parameters using @param, @return, > and @throws Javadoc tags, > where applicable. > For internal API, JavaDoc comments should be optional. It's up to a > contributor or reviewer. > > What are you think? > > 1. > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments signature.asc Description: This is a digitally signed message part
Re: [EXTERNAL] Re: Replace or Put after PutAsync causes Ignite to hang
Hello!
+ dev@
I think the current behavior, where .Net callbacks may be run from striped
pool, violate some of Ignite assumptions: that we never run user code from
certain thread pools (like sys-stripe) and that we try to limit options of
running user-supplied code from our internals.
I think that future versions of .Net integration should remove the ability
of async callbacks to be called from non-user threads, even if it can lead
to performance degradation in some cases. I suggest removing this mode, if
possible, while keeping only the safe one, where internal threads are not
waiting upon completion of user code.
In this case my issue IGNITE-12033 could be used to track this work.
WDYT?
Regards,
--
Ilya Kasnacheev
ср, 7 авг. 2019 г. в 01:47, Pavel Tupitsyn :
> Sorry guys, I've completely missed this thread, and the topic is very
> important.
>
> First, a simple fix for the given example. Add the following on the first
> line of Main:
> SynchronizationContext.SetSynchronizationContext(new
> ThreadPoolSynchronizationContext());
>
> And put the ThreadPoolSynchronizationContext class somewhere:
> class ThreadPoolSynchronizationContext : SynchronizationContext
> {
> // No-op.
> }
>
>
> Now, detailed explanation. The problem exists forever in Ignite and is
> mentioned in the docs briefly [1].
> Also mentioned in .NET docs (I've updated them a bit) [2].
>
> Breakdown:
> * Ignite (Java side) runs async callbacks (continuations) on system
> threads, and those threads have limitations (you should not call Ignite
> APIs from them in general)
> * Ignite.NET wraps async operations into native .NET Tasks
> * Usually `await ...` call in .NET will continue execution on the original
> Thread (simply put, actually it is more complex), so Ignite system thread
> issue is avoided
> * However, Console applications have no `SynchronizationContext`, so the
> continuation can't be dispatched to original thread, and is executed on
> current (Ignite) thread
> * Setting custom SynchronizationContext fixes the issue: all async
> continuations will be dispatched to .NET thread pool and never executed on
> Ignite threads
>
> However, dispatching callbacks to a different thread causes performance
> hit, and Ignite favors performance over usability right now.
> So it is up to the user to configure desired behavior.
>
> Let me know if you need more details.
>
> Thanks
>
> [1] https://apacheignite.readme.io/docs/async-support
> [2] https://apacheignite-net.readme.io/docs/asynchronous-support
>
> On Thu, Aug 1, 2019 at 3:41 PM Ilya Kasnacheev
> wrote:
>
>> Hello!
>>
>> I have filed a ticket about this issue so it won't get lost.
>> https://issues.apache.org/jira/browse/IGNITE-12033
>>
>> Regards,
>> --
>> Ilya Kasnacheev
>>
>>
>> чт, 2 мая 2019 г. в 10:53, Barney Pippin > >:
>>
>>> Thanks for the response Ilya. Did you get a chance to look at this Pavel?
>>> Thanks.
>>>
>>>
>>>
>>> --
>>> Sent from: http://apache-ignite-users.70518.x6.nabble.com/
>>>
>>
Coding guidelines. Useless JavaDoc comments.
Igniters! I think it's time to change coding guidelines in part of JavaDoc comments [1]: >> Every method, field or initializer public, private or protected in top-level, >> inner or anonymous type should have at least minimal Javadoc comments including >> description and description of parameters using @param, @return and @throws Javadoc tags, >> where applicable. Let's look at JavaDoc comments in the IgniteKernal class: Why? /** */ - 15 matches. What can you get new from these comments? /** Periodic starvation check interval. */ private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30; /** Long jvm pause detector. */ private LongJVMPauseDetector longJVMPauseDetector; /** Scheduler. */ private IgniteScheduler scheduler; /** Stop guard. */ private final AtomicBoolean stopGuard = new AtomicBoolean(); /** * @param cfg Configuration to use. * @param utilityCachePool Utility cache pool. * @param execSvc Executor service. * @param sysExecSvc System executor service. * @param stripedExecSvc Striped executor. * @param p2pExecSvc P2P executor service. * @param mgmtExecSvc Management executor service. * @param igfsExecSvc IGFS executor service. * @param dataStreamExecSvc data stream executor service. * @param restExecSvc Reset executor service. * @param affExecSvc Affinity executor service. * @param idxExecSvc Indexing executor service. * @param callbackExecSvc Callback executor service. * @param qryExecSvc Query executor service. * @param schemaExecSvc Schema executor service. * @param customExecSvcs Custom named executors. * @param errHnd Error handler to use for notification about startup problems. * @param workerRegistry Worker registry. * @param hnd Default uncaught exception handler used by thread pools. * @throws IgniteCheckedException Thrown in case of any errors. */ public void start( final IgniteConfiguration cfg, ExecutorService utilityCachePool, final ExecutorService execSvc, final ExecutorService svcExecSvc, final ExecutorService sysExecSvc, final StripedExecutor stripedExecSvc, ExecutorService p2pExecSvc, ExecutorService mgmtExecSvc, ExecutorService igfsExecSvc, StripedExecutor dataStreamExecSvc, ExecutorService restExecSvc, ExecutorService affExecSvc, @Nullable ExecutorService idxExecSvc, IgniteStripedThreadPoolExecutor callbackExecSvc, ExecutorService qryExecSvc, ExecutorService schemaExecSvc, @Nullable final Map customExecSvcs, GridAbsClosure errHnd, WorkersRegistry workerRegistry, Thread.UncaughtExceptionHandler hnd, TimeBag startTimer ) These comments look ugly and useless, and that is the main class of core. Why do they need us? Let us change the coding guidelines in part of JavaDoc comments: Every method public API should have at least minimal Javadoc comments, including description and description of parameters using @param, @return, and @throws Javadoc tags, where applicable. For internal API, JavaDoc comments should be optional. It's up to a contributor or reviewer. What are you think? 1. https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
Re: [DISCUSSION][IEP-35] Metrics configuration
This is a tough topic, however, in my opinion, static bucket configuration for histograms will work worse than no configuration at all. The reason is simple - complexity. If we assume a need to change bucket configuration for some metric, we can assume that this metric can be related to a cache. Now, we can assume that different caches require different metric configuration. Combined with the ability to start dynamic caches, we get no simple way to change the metric for dynamically started caches. >From my experience, properly chosen buckets are usually good enough to troubleshoot most of the cases. For advanced scenarios, I would delay the configuration exposure and designed a properly structured control.sh commands which will utilize either metastore or whatever another system we choose to keep this configuration. Managing configuration files in large clusters is a big pain, no need to make it worse. вт, 6 авг. 2019 г. в 15:32, Павлухин Иван : > Andrey, > > It seems that screenshot was rejected, I do not see it. > > вт, 6 авг. 2019 г. в 15:07, Andrey Gura : > > > > Good example of proper buckets configuration. Such configuration is > > suitable for may cases. See attached screenshot (I hope it will not be > > reject by mail system or forum). > > > > > > On Tue, Aug 6, 2019 at 2:45 PM Andrey Gura wrote: > > > > > > > What do you mean by "exponential bounds"? > > > > > > Something like this if we talk about latency in ms for example: 5, 10, > > > 25, 50, 100, 200, 500, ... > > > > > > > Thanks, for the feedback, appreciate you ownesty. > > > > > > Nothing personal. It is just about functionality from user's stand > point. > > > > > > > What is your proposal? > > > > How metrics configuration should work? > > > > > > My proposal is simple: just drop this change. We don't need the > > > configuration. Metric owner (developer) defines buckets' bounds for > > > each particular case (it could be done uniformly or exponentially, it > > > depends on metric and problem definition). > > > > > > On Mon, Aug 5, 2019 at 6:36 PM Nikolay Izhikov > wrote: > > > > > > > > Hello, Andrey. > > > > > > > > > Not necessary if we have exponential bounds' values for histograms. > > > > > > > > What do you mean by "exponential bounds"? > > > > > > > > > Anyway, in current solution it looks ugly and not usable. > > > > > > > > Thanks, for the feedback, appreciate you ownesty. > > > > > > > > > No. But we should admit that this is bad decision and do not > include this change to the code base. > > > > > > > > What is your proposal? > > > > How metrics configuration should work? > > > > > > > > > Yes. But it still will not give enough accuracy. > > > > > > > > Enough for what? > > > > > > > > В Пн, 05/08/2019 в 18:29 +0300, Andrey Gura пишет: > > > > > > > - metric configuration is node local (not cluster wide). > > > > > > This issue is easy to solve on the user-side and in Ignite core. > > > > > > > > > > It's imaginary simplicity. The first, you need some additional > > > > > automation on user-side in order to configure all nodes of the > > > > > cluster. The second, new nodes can join to the cluster and > > > > > configuration will be different on new node and on other nodes of > the > > > > > cluster. This leads to complication whole functionality. Anyway, I > > > > > don't like such simplified solution because at the moment it brings > > > > > more problems than value. > > > > > > > > > > > The easiest solution was implemented. > > > > > > Do we want to make it more complex right now :)? > > > > > > > > > > No. But we should admit that this is bad decision and do not > include > > > > > this change to the code base. > > > > > > > > > > > The reason it exists in PR - we already have this parameter in > DataStorageConfiguration#getMetricsSubIntervalCount > > > > > > > > > > I believe this method should be deprecated and removed in major > release. > > > > > > > > > > > I think the user should be able to configure buckets for > histogram and rateTimeInterval for hitrate. > > > > > > > > > > Not necessary if we have exponential bounds' values for histograms. > > > > > Anyway, in current solution it looks ugly and not usable. > > > > > > > > > > > Ignite has dozens of use-cases and deployment modes, seems, > > > > > > we can't cover it all with the single predefined > buckets/rateTimeInterval set. > > > > > > > > > > Yes. But it still will not give enough accuracy. > > > > > > > > > > On Mon, Aug 5, 2019 at 5:25 PM Nikolay Izhikov < > nizhi...@apache.org> wrote: > > > > > > > > > > > > Hello, Andrey. > > > > > > > > > > > > > - metric configuration is node local (not cluster wide). > > > > > > > > > > > > This issue is easy to solve on the user-side and in Ignite core. > > > > > > > > > > > > > - metric configuration doesn't survive node restart. > > > > > > > > > > > > We decide to go with the simplest solution, for now. > > > > > > The easiest solution was implemented. > > > > > > Do we want to make it more complex right
Re: New Ignite PMC Member: Ilya Kasnacheev
Hello! I hope to uphold your trust. Thanks everyone for your support! Regards, -- Ilya Kasnacheev ср, 7 авг. 2019 г. в 09:36, Vyacheslav Daradur : > Ilya, congratulations! > > On Wed, Aug 7, 2019 at 8:41 AM Павлухин Иван wrote: > > > > Ilya, my congratulations! > > > > вт, 6 авг. 2019 г. в 23:12, Dmitriy Pavlov : > > > > > > Ilya, congratulations! > > > > > > вт, 6 авг. 2019 г. в 22:58, Denis Magda : > > > > > > > The Project Management Committee (PMC) for Apache Ignite > > > > has invited Ilya Kasnacheev to become a committer and we are pleased > > > > to announce that he has accepted. > > > > > > > > Ilya is one of the most active and valuable Ignite committers who is > not > > > > only contributing source-code changes but also takes an active > stance on > > > > Ignite adoption - he constantly helps Ignite users to design, > troubleshoot > > > > and optimize their Ignite deployments. > > > > > > > > Being a PMC member enables assistance with the management and to > guide the > > > > direction of the project. > > > > > > > > Please join me in congratulating Ilya with this new role! > > > > > > > > > > > > - > > > > Denis > > > > > > > > > > > > -- > > Best regards, > > Ivan Pavlukhin > > > > -- > Best Regards, Vyacheslav D. >
Re: New Ignite PMC Member: Ilya Kasnacheev
Ilya, congratulations! On Wed, Aug 7, 2019 at 8:41 AM Павлухин Иван wrote: > > Ilya, my congratulations! > > вт, 6 авг. 2019 г. в 23:12, Dmitriy Pavlov : > > > > Ilya, congratulations! > > > > вт, 6 авг. 2019 г. в 22:58, Denis Magda : > > > > > The Project Management Committee (PMC) for Apache Ignite > > > has invited Ilya Kasnacheev to become a committer and we are pleased > > > to announce that he has accepted. > > > > > > Ilya is one of the most active and valuable Ignite committers who is not > > > only contributing source-code changes but also takes an active stance on > > > Ignite adoption - he constantly helps Ignite users to design, troubleshoot > > > and optimize their Ignite deployments. > > > > > > Being a PMC member enables assistance with the management and to guide the > > > direction of the project. > > > > > > Please join me in congratulating Ilya with this new role! > > > > > > > > > - > > > Denis > > > > > > > -- > Best regards, > Ivan Pavlukhin -- Best Regards, Vyacheslav D.
Re: SecurityTestSuite as a separate test suite at TC
Hi Denis. I think it is fine to extract security tests in a separate build plan on TC. BTW, if you are going to write a lot of Sandbox's tests pay attention to 'extdata' module and an approach of P2P tests (IgniteP2PSelfTestSuite) - this may help you to avoid Maven's classloading issues. On Tue, Aug 6, 2019 at 3:25 PM Denis Garus wrote: > > Hello Igniters! > > I made the test DoPrivelegedOnRemoteNodeTest[1] (SecurityTestSuite) for the > task "Sandbox for user-defined code" [2] > that uses p2p deploy like the test > ServiceHotRedeploymentViaDeploymentSpiTest [3] from > IgniteServiceGridTestSuite. > That test requires additional Maven command line parameter -P > surefire-fork-count-1. > The suite Basic 1 contains the SecurityTestSuite and many other test suites > at TeamCity that do not need that additional Maven parameter. > I suggest extracting SecurityTestSuite as a separate test suite to define > additional Maven command line parameter for it. > > WDYT? > > > 1. https://github.com/apache/ignite/pull/6707 > 2. https://issues.apache.org/jira/browse/IGNITE-11410 > 3. > https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/service/ServiceHotRedeploymentViaDeploymentSpiTest.java -- Best Regards, Vyacheslav D.
