Re: Transaction classes naming

2017-12-21 Thread Andrey Gura
Completely agree. Also locks acquisition related futures should be renamed.

On Thu, Dec 21, 2017 at 8:21 AM, ALEKSEY KUZNETSOV
 wrote:
>
> for renaming +1
>
>> 21 дек. 2017 г., в 5:25, Dmitriy Setrakyan  
>> написал(а):
>>
>> Will this renaming introduce any issues with supporting previous versions?
>>
>>> On Wed, Dec 20, 2017 at 9:10 AM, Yakov Zhdanov  wrote:
>>>
>>> How about we explain it in javadocs and properly document on wiki? Guys, I
>>> would do this at some point when there is no another bigger issue in Ignite
>>> =)
>>>
>>> --Yakov
>>>


Re: Transaction classes naming

2017-12-20 Thread ALEKSEY KUZNETSOV

for renaming +1

> 21 дек. 2017 г., в 5:25, Dmitriy Setrakyan  написал(а):
> 
> Will this renaming introduce any issues with supporting previous versions?
> 
>> On Wed, Dec 20, 2017 at 9:10 AM, Yakov Zhdanov  wrote:
>> 
>> How about we explain it in javadocs and properly document on wiki? Guys, I
>> would do this at some point when there is no another bigger issue in Ignite
>> =)
>> 
>> --Yakov
>> 


Re: Transaction classes naming

2017-12-20 Thread Dmitriy Setrakyan
Will this renaming introduce any issues with supporting previous versions?

On Wed, Dec 20, 2017 at 9:10 AM, Yakov Zhdanov  wrote:

> How about we explain it in javadocs and properly document on wiki? Guys, I
> would do this at some point when there is no another bigger issue in Ignite
> =)
>
> --Yakov
>


Re: Transaction classes naming

2017-12-20 Thread Yakov Zhdanov
How about we explain it in javadocs and properly document on wiki? Guys, I
would do this at some point when there is no another bigger issue in Ignite
=)

--Yakov


Re: Transaction classes naming

2017-12-20 Thread Alexey Goncharuk
Andrey,

Good point. Even though it is not as confusing as near-remote naming, I
agree that this should be fixed as well.

2017-12-20 18:59 GMT+03:00 Andrey Kuznetsov <stku...@gmail.com>:

> Alexey,
>
> And what do you think about 'Adapters' in transaction heirarchy? This term
> looks really obfuscating for new contributor, since in fact they are not
> adapters for something but abstract superclasses.
>
> 2017-12-20 18:18 GMT+03:00 Alexey Goncharuk <alexey.goncha...@gmail.com>:
>
> > Igniters,
> >
> > As I review transaction-related PRs and communicate with other fellow
> > Igniters, I realized that our transaction classes naming became
> > inconsistent and almost impossible to explain to a new contributor. A few
> > examples:
> >
> > 1) We have a GridNearTxLocal class, but Near in the name does not
> > necessarily mean that this transaction has to do with the near cache. It
> > may or may not, depending on the cache configuration. Near in this case
> > only stands for _originating_ node transaction. This is also reflected in
> > many messages, such as Near*Request, where near stands for a message sent
> > from an originating node to the primary node.
> > 2) We have GridNearTxRemote and GridDhtTxRemote classes. First, near here
> > always means near cache, as near remote transaction will only be created
> > for near-enabled caches. Second, Remote in the class names stands for
> > backups (affinity backups or temporarily near-reader backups). On the
> other
> > hand, requests sent from primary to backups are named Dht*Request.
> >
> > All in all, the naming is extremely confusing. For a person who hasn't
> seen
> > the evolution of transaction classes over time, it is impossible to infer
> > or even assume what these classes stand for.
> >
> > I would like to kick off a discussion on new transaction classes and
> > messages naming (especially as we started developing MVCC which most
> likely
> > will introduce additional TX classes).
> >
> > How about:
> > OriginatingTx for transaction created on originating node
> > PrimaryTx for transaction created on primary nodes, but not the
> originating
> > node
> > BackupTx for transaction created on backup nodes
> > NearTx for transaction created to update near cache
> >
> > Thoughts?
> >
>
>
>
> --
> Best regards,
>   Andrey Kuznetsov.
>


Re: Transaction classes naming

2017-12-20 Thread Andrey Kuznetsov
Alexey,

And what do you think about 'Adapters' in transaction heirarchy? This term
looks really obfuscating for new contributor, since in fact they are not
adapters for something but abstract superclasses.

2017-12-20 18:18 GMT+03:00 Alexey Goncharuk <alexey.goncha...@gmail.com>:

> Igniters,
>
> As I review transaction-related PRs and communicate with other fellow
> Igniters, I realized that our transaction classes naming became
> inconsistent and almost impossible to explain to a new contributor. A few
> examples:
>
> 1) We have a GridNearTxLocal class, but Near in the name does not
> necessarily mean that this transaction has to do with the near cache. It
> may or may not, depending on the cache configuration. Near in this case
> only stands for _originating_ node transaction. This is also reflected in
> many messages, such as Near*Request, where near stands for a message sent
> from an originating node to the primary node.
> 2) We have GridNearTxRemote and GridDhtTxRemote classes. First, near here
> always means near cache, as near remote transaction will only be created
> for near-enabled caches. Second, Remote in the class names stands for
> backups (affinity backups or temporarily near-reader backups). On the other
> hand, requests sent from primary to backups are named Dht*Request.
>
> All in all, the naming is extremely confusing. For a person who hasn't seen
> the evolution of transaction classes over time, it is impossible to infer
> or even assume what these classes stand for.
>
> I would like to kick off a discussion on new transaction classes and
> messages naming (especially as we started developing MVCC which most likely
> will introduce additional TX classes).
>
> How about:
> OriginatingTx for transaction created on originating node
> PrimaryTx for transaction created on primary nodes, but not the originating
> node
> BackupTx for transaction created on backup nodes
> NearTx for transaction created to update near cache
>
> Thoughts?
>



-- 
Best regards,
  Andrey Kuznetsov.


Re: Transaction classes naming

2017-12-20 Thread Ivan Rakov
As a person who tried to infer transactional behavior from listed 
classes, I completely agree.


Also, I'd like to pay additional attention to:

1) IgniteTxAdapter#near method. It returns true both for GridNearTxLocal 
(where "near" means originating node) and GridNearTxRemote (where "near" 
means near cache). This is extremely confusing, we should rename this 
method as well.


2) GridNear*Request/Response classes (including non-transactional ones). 
They incapsulate requests from originating to primary nodes, so we 
should consider renaming them to GridOriginating*Request/Response.


Best Regards,
Ivan Rakov

On 20.12.2017 18:18, Alexey Goncharuk wrote:

Igniters,

As I review transaction-related PRs and communicate with other fellow
Igniters, I realized that our transaction classes naming became
inconsistent and almost impossible to explain to a new contributor. A few
examples:

1) We have a GridNearTxLocal class, but Near in the name does not
necessarily mean that this transaction has to do with the near cache. It
may or may not, depending on the cache configuration. Near in this case
only stands for _originating_ node transaction. This is also reflected in
many messages, such as Near*Request, where near stands for a message sent
from an originating node to the primary node.
2) We have GridNearTxRemote and GridDhtTxRemote classes. First, near here
always means near cache, as near remote transaction will only be created
for near-enabled caches. Second, Remote in the class names stands for
backups (affinity backups or temporarily near-reader backups). On the other
hand, requests sent from primary to backups are named Dht*Request.

All in all, the naming is extremely confusing. For a person who hasn't seen
the evolution of transaction classes over time, it is impossible to infer
or even assume what these classes stand for.

I would like to kick off a discussion on new transaction classes and
messages naming (especially as we started developing MVCC which most likely
will introduce additional TX classes).

How about:
OriginatingTx for transaction created on originating node
PrimaryTx for transaction created on primary nodes, but not the originating
node
BackupTx for transaction created on backup nodes
NearTx for transaction created to update near cache

Thoughts?





Re: Transaction classes naming

2017-12-20 Thread Vladimir Ozerov
+1

On Wed, Dec 20, 2017 at 6:18 PM, Alexey Goncharuk <
alexey.goncha...@gmail.com> wrote:

> Igniters,
>
> As I review transaction-related PRs and communicate with other fellow
> Igniters, I realized that our transaction classes naming became
> inconsistent and almost impossible to explain to a new contributor. A few
> examples:
>
> 1) We have a GridNearTxLocal class, but Near in the name does not
> necessarily mean that this transaction has to do with the near cache. It
> may or may not, depending on the cache configuration. Near in this case
> only stands for _originating_ node transaction. This is also reflected in
> many messages, such as Near*Request, where near stands for a message sent
> from an originating node to the primary node.
> 2) We have GridNearTxRemote and GridDhtTxRemote classes. First, near here
> always means near cache, as near remote transaction will only be created
> for near-enabled caches. Second, Remote in the class names stands for
> backups (affinity backups or temporarily near-reader backups). On the other
> hand, requests sent from primary to backups are named Dht*Request.
>
> All in all, the naming is extremely confusing. For a person who hasn't seen
> the evolution of transaction classes over time, it is impossible to infer
> or even assume what these classes stand for.
>
> I would like to kick off a discussion on new transaction classes and
> messages naming (especially as we started developing MVCC which most likely
> will introduce additional TX classes).
>
> How about:
> OriginatingTx for transaction created on originating node
> PrimaryTx for transaction created on primary nodes, but not the originating
> node
> BackupTx for transaction created on backup nodes
> NearTx for transaction created to update near cache
>
> Thoughts?
>


Transaction classes naming

2017-12-20 Thread Alexey Goncharuk
Igniters,

As I review transaction-related PRs and communicate with other fellow
Igniters, I realized that our transaction classes naming became
inconsistent and almost impossible to explain to a new contributor. A few
examples:

1) We have a GridNearTxLocal class, but Near in the name does not
necessarily mean that this transaction has to do with the near cache. It
may or may not, depending on the cache configuration. Near in this case
only stands for _originating_ node transaction. This is also reflected in
many messages, such as Near*Request, where near stands for a message sent
from an originating node to the primary node.
2) We have GridNearTxRemote and GridDhtTxRemote classes. First, near here
always means near cache, as near remote transaction will only be created
for near-enabled caches. Second, Remote in the class names stands for
backups (affinity backups or temporarily near-reader backups). On the other
hand, requests sent from primary to backups are named Dht*Request.

All in all, the naming is extremely confusing. For a person who hasn't seen
the evolution of transaction classes over time, it is impossible to infer
or even assume what these classes stand for.

I would like to kick off a discussion on new transaction classes and
messages naming (especially as we started developing MVCC which most likely
will introduce additional TX classes).

How about:
OriginatingTx for transaction created on originating node
PrimaryTx for transaction created on primary nodes, but not the originating
node
BackupTx for transaction created on backup nodes
NearTx for transaction created to update near cache

Thoughts?