You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Zhenya Stanilovsky <ar...@mail.ru.INVALID> on 2020/05/25 11:53:28 UTC

Re[4]: IGNITE-6499 Compact NULL fields

Compress of whole binary inside ignite.


 
>Sorry I do not actual get what are you opposing? the compress of the binary
>or the null compaction or both?
>And can you ellaborate on why you are opposing it?
>
>
>
>--
>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/ 
 
 
 
 

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

Yes, I think this is a sensible approach.

Regards,
-- 
Ilya Kasnacheev


ср, 8 июл. 2020 г. в 14:46, Ivan Daschinsky <iv...@gmail.com>:

> I think that this feature can be handled as compactFooter. For example, C++
> doesn't support compactFooter and it is not an issue.
> Of course, this feature should be disabled by default, and should be
> enabled explicitly in BinaryConfiguration.
> Also, subsequent issues in jira about this feature support in platforms
> should be created.
>
> ср, 8 июл. 2020 г. в 14:31, Ilya Kasnacheev <il...@gmail.com>:
>
> > Hello!
> >
> > I think this is a blocker for this change. We already have binary format
> > published:
> >
> >
> https://apacheignite.readme.io/docs/binary-client-protocol-data-format#complex-object
> >
> > Arguably, we cannot change it in a minor version of Apache Ignite, so
> this
> > change has to target AI 3.0.
> >
> > Extending this binary format with e.g. new operations could probably be
> OK.
> > But we have clients released on a different schedule in their own repos
> > (and there are some 3rd party clients too), we can't release a minor
> > version which will change this format unilaterally without any change of
> > operation (same data, same calls, different result after upgrade, broken
> > clients).
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 8 июл. 2020 г. в 13:43, Ivan Daschinsky <iv...@gmail.com>:
> >
> > > Hi!
> > > Ilya, unfortunatelly yes, subsequent changes should be made in C++,
> .NET
> > > and other platform code.
> > >
> > > ср, 8 июл. 2020 г. в 12:22, Ilya Kasnacheev <ilya.kasnacheev@gmail.com
> >:
> > >
> > > > Hello fellow devs,
> > > >
> > > > I just wanted to ask, how would this Binary Object format change
> affect
> > > > thin clients? C++/.Net nodes? Etc?
> > > >
> > > > Is it fully backwards compatible or not?
> > > >
> > > > I think that realistically, we can only add binary-incompatible
> changes
> > > to
> > > > Binary Object format in 3.0.
> > > >
> > > > Regards,
> > > > --
> > > > Ilya Kasnacheev
> > > >
> > > >
> > > > ср, 8 июл. 2020 г. в 09:05, Ivan Pavlukhin <vo...@gmail.com>:
> > > >
> > > > > A side note. Now we have a neat URL for TC bot
> > > > > https://mtcga.ignite.apache.org/ (along with one in a gridgain
> > > > > domain).
> > > > >
> > > > > 2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky
> > > > <arzamas123@mail.ru.invalid
> > > > > >:
> > > > > >
> > > > > > request it, check for example [1]
> > > > > >
> > > > > > also you need to run [2] tests.
> > > > > >
> > > > > > [1]
> > > > > >
> > > > >
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> > > > > > [2] https://mtcga.gridgain.com
> > > > > >>Hello,
> > > > > >>
> > > > > >>Look at the ticket and the only comment I can see is creating a
> > > branch
> > > > on
> > > > > >>git in the main repo and not in my fork. I do not have the right
> to
> > > > > create
> > > > > >> a
> > > > > >>branch in the main repository. Am i missing something?
> > > > > >>
> > > > > >>Sorry I probably misread the document but I though that I should
> > fork
> > > > the
> > > > > >>repo and then pull request as I do not have the rights to create
> a
> > > > > branch.
> > > > > >>
> > > > > >>Thanks for your help
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >>--
> > > > > >>Sent from:
> http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > Best regards,
> > > > > Ivan Pavlukhin
> > > > >
> > > >
> > >
> > >
> > > --
> > > Sincerely yours, Ivan Daschinskiy
> > >
> >
>
>
> --
> Sincerely yours, Ivan Daschinskiy
>

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ivan Daschinsky <iv...@gmail.com>.
I think that this feature can be handled as compactFooter. For example, C++
doesn't support compactFooter and it is not an issue.
Of course, this feature should be disabled by default, and should be
enabled explicitly in BinaryConfiguration.
Also, subsequent issues in jira about this feature support in platforms
should be created.

ср, 8 июл. 2020 г. в 14:31, Ilya Kasnacheev <il...@gmail.com>:

> Hello!
>
> I think this is a blocker for this change. We already have binary format
> published:
>
> https://apacheignite.readme.io/docs/binary-client-protocol-data-format#complex-object
>
> Arguably, we cannot change it in a minor version of Apache Ignite, so this
> change has to target AI 3.0.
>
> Extending this binary format with e.g. new operations could probably be OK.
> But we have clients released on a different schedule in their own repos
> (and there are some 3rd party clients too), we can't release a minor
> version which will change this format unilaterally without any change of
> operation (same data, same calls, different result after upgrade, broken
> clients).
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 8 июл. 2020 г. в 13:43, Ivan Daschinsky <iv...@gmail.com>:
>
> > Hi!
> > Ilya, unfortunatelly yes, subsequent changes should be made in C++, .NET
> > and other platform code.
> >
> > ср, 8 июл. 2020 г. в 12:22, Ilya Kasnacheev <il...@gmail.com>:
> >
> > > Hello fellow devs,
> > >
> > > I just wanted to ask, how would this Binary Object format change affect
> > > thin clients? C++/.Net nodes? Etc?
> > >
> > > Is it fully backwards compatible or not?
> > >
> > > I think that realistically, we can only add binary-incompatible changes
> > to
> > > Binary Object format in 3.0.
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> > >
> > >
> > > ср, 8 июл. 2020 г. в 09:05, Ivan Pavlukhin <vo...@gmail.com>:
> > >
> > > > A side note. Now we have a neat URL for TC bot
> > > > https://mtcga.ignite.apache.org/ (along with one in a gridgain
> > > > domain).
> > > >
> > > > 2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky
> > > <arzamas123@mail.ru.invalid
> > > > >:
> > > > >
> > > > > request it, check for example [1]
> > > > >
> > > > > also you need to run [2] tests.
> > > > >
> > > > > [1]
> > > > >
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> > > > > [2] https://mtcga.gridgain.com
> > > > >>Hello,
> > > > >>
> > > > >>Look at the ticket and the only comment I can see is creating a
> > branch
> > > on
> > > > >>git in the main repo and not in my fork. I do not have the right to
> > > > create
> > > > >> a
> > > > >>branch in the main repository. Am i missing something?
> > > > >>
> > > > >>Sorry I probably misread the document but I though that I should
> fork
> > > the
> > > > >>repo and then pull request as I do not have the rights to create a
> > > > branch.
> > > > >>
> > > > >>Thanks for your help
> > > > >>
> > > > >>
> > > > >>
> > > > >>--
> > > > >>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Best regards,
> > > > Ivan Pavlukhin
> > > >
> > >
> >
> >
> > --
> > Sincerely yours, Ivan Daschinskiy
> >
>


-- 
Sincerely yours, Ivan Daschinskiy

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

If we do improve it, I think we should go for a full re-think as opposed to
a single breaking change that doesn't actually improve that much.

Nevertheless, I think we can commit some improvements with opt-in
BinaryConfiguration.

Regards,
-- 
Ilya Kasnacheev


сб, 11 июл. 2020 г. в 01:27, steve.hostettler@gmail.com <
steve.hostettler@gmail.com>:

> Ok gotcha, so it is not going to make it.
>
> Just to note that we are dragging this since before v2.0 and just a
> reminder
> that someone else tried a similar thing before v2 and it got blocked
> because
> it was too much of a change for v2.
>
> Typically the type of things that we can never change because it is too
> much
> of an impact.
>
> BinaryObject format is not optimal far for it and if we cannot change it
> incrementally nor in a big bang...
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by "steve.hostettler@gmail.com" <st...@gmail.com>.
Ok gotcha, so it is not going to make it. 

Just to note that we are dragging this since before v2.0 and just a reminder
that someone else tried a similar thing before v2 and it got blocked because
it was too much of a change for v2.

Typically the type of things that we can never change because it is too much
of an impact.

BinaryObject format is not optimal far for it and if we cannot change it
incrementally nor in a big bang...



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

I think this is a blocker for this change. We already have binary format
published:
https://apacheignite.readme.io/docs/binary-client-protocol-data-format#complex-object

Arguably, we cannot change it in a minor version of Apache Ignite, so this
change has to target AI 3.0.

Extending this binary format with e.g. new operations could probably be OK.
But we have clients released on a different schedule in their own repos
(and there are some 3rd party clients too), we can't release a minor
version which will change this format unilaterally without any change of
operation (same data, same calls, different result after upgrade, broken
clients).

Regards,
-- 
Ilya Kasnacheev


ср, 8 июл. 2020 г. в 13:43, Ivan Daschinsky <iv...@gmail.com>:

> Hi!
> Ilya, unfortunatelly yes, subsequent changes should be made in C++, .NET
> and other platform code.
>
> ср, 8 июл. 2020 г. в 12:22, Ilya Kasnacheev <il...@gmail.com>:
>
> > Hello fellow devs,
> >
> > I just wanted to ask, how would this Binary Object format change affect
> > thin clients? C++/.Net nodes? Etc?
> >
> > Is it fully backwards compatible or not?
> >
> > I think that realistically, we can only add binary-incompatible changes
> to
> > Binary Object format in 3.0.
> >
> > Regards,
> > --
> > Ilya Kasnacheev
> >
> >
> > ср, 8 июл. 2020 г. в 09:05, Ivan Pavlukhin <vo...@gmail.com>:
> >
> > > A side note. Now we have a neat URL for TC bot
> > > https://mtcga.ignite.apache.org/ (along with one in a gridgain
> > > domain).
> > >
> > > 2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky
> > <arzamas123@mail.ru.invalid
> > > >:
> > > >
> > > > request it, check for example [1]
> > > >
> > > > also you need to run [2] tests.
> > > >
> > > > [1]
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> > > > [2] https://mtcga.gridgain.com
> > > >>Hello,
> > > >>
> > > >>Look at the ticket and the only comment I can see is creating a
> branch
> > on
> > > >>git in the main repo and not in my fork. I do not have the right to
> > > create
> > > >> a
> > > >>branch in the main repository. Am i missing something?
> > > >>
> > > >>Sorry I probably misread the document but I though that I should fork
> > the
> > > >>repo and then pull request as I do not have the rights to create a
> > > branch.
> > > >>
> > > >>Thanks for your help
> > > >>
> > > >>
> > > >>
> > > >>--
> > > >>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Ivan Pavlukhin
> > >
> >
>
>
> --
> Sincerely yours, Ivan Daschinskiy
>

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ivan Daschinsky <iv...@gmail.com>.
Hi!
Ilya, unfortunatelly yes, subsequent changes should be made in C++, .NET
and other platform code.

ср, 8 июл. 2020 г. в 12:22, Ilya Kasnacheev <il...@gmail.com>:

> Hello fellow devs,
>
> I just wanted to ask, how would this Binary Object format change affect
> thin clients? C++/.Net nodes? Etc?
>
> Is it fully backwards compatible or not?
>
> I think that realistically, we can only add binary-incompatible changes to
> Binary Object format in 3.0.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> ср, 8 июл. 2020 г. в 09:05, Ivan Pavlukhin <vo...@gmail.com>:
>
> > A side note. Now we have a neat URL for TC bot
> > https://mtcga.ignite.apache.org/ (along with one in a gridgain
> > domain).
> >
> > 2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky
> <arzamas123@mail.ru.invalid
> > >:
> > >
> > > request it, check for example [1]
> > >
> > > also you need to run [2] tests.
> > >
> > > [1]
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> > > [2] https://mtcga.gridgain.com
> > >>Hello,
> > >>
> > >>Look at the ticket and the only comment I can see is creating a branch
> on
> > >>git in the main repo and not in my fork. I do not have the right to
> > create
> > >> a
> > >>branch in the main repository. Am i missing something?
> > >>
> > >>Sorry I probably misread the document but I though that I should fork
> the
> > >>repo and then pull request as I do not have the rights to create a
> > branch.
> > >>
> > >>Thanks for your help
> > >>
> > >>
> > >>
> > >>--
> > >>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> > >
> > >
> > >
> >
> >
> > --
> >
> > Best regards,
> > Ivan Pavlukhin
> >
>


-- 
Sincerely yours, Ivan Daschinskiy

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello fellow devs,

I just wanted to ask, how would this Binary Object format change affect
thin clients? C++/.Net nodes? Etc?

Is it fully backwards compatible or not?

I think that realistically, we can only add binary-incompatible changes to
Binary Object format in 3.0.

Regards,
-- 
Ilya Kasnacheev


ср, 8 июл. 2020 г. в 09:05, Ivan Pavlukhin <vo...@gmail.com>:

> A side note. Now we have a neat URL for TC bot
> https://mtcga.ignite.apache.org/ (along with one in a gridgain
> domain).
>
> 2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky <arzamas123@mail.ru.invalid
> >:
> >
> > request it, check for example [1]
> >
> > also you need to run [2] tests.
> >
> > [1]
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> > [2] https://mtcga.gridgain.com
> >>Hello,
> >>
> >>Look at the ticket and the only comment I can see is creating a branch on
> >>git in the main repo and not in my fork. I do not have the right to
> create
> >> a
> >>branch in the main repository. Am i missing something?
> >>
> >>Sorry I probably misread the document but I though that I should fork the
> >>repo and then pull request as I do not have the rights to create a
> branch.
> >>
> >>Thanks for your help
> >>
> >>
> >>
> >>--
> >>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/
> >
> >
> >
> >
>
>
> --
>
> Best regards,
> Ivan Pavlukhin
>

Re: Re[2]: IGNITE-6499 Compact NULL fields

Posted by Ivan Pavlukhin <vo...@gmail.com>.
A side note. Now we have a neat URL for TC bot
https://mtcga.ignite.apache.org/ (along with one in a gridgain
domain).

2020-07-07 18:43 GMT+03:00, Zhenya Stanilovsky <ar...@mail.ru.invalid>:
>
> request it, check for example [1]
>
> also you need to run [2] tests.
>
> [1]
> http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
> [2] https://mtcga.gridgain.com
>>Hello,
>>
>>Look at the ticket and the only comment I can see is creating a branch on
>>git in the main repo and not in my fork. I do not have the right to create
>> a
>>branch in the main repository. Am i missing something?
>>
>>Sorry I probably misread the document but I though that I should fork the
>>repo and then pull request as I do not have the rights to create a branch.
>>
>>Thanks for your help
>>
>>
>>
>>--
>>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/
>
>
>
>


-- 

Best regards,
Ivan Pavlukhin

Re[2]: IGNITE-6499 Compact NULL fields

Posted by Zhenya Stanilovsky <ar...@mail.ru.INVALID>.
request it, check for example [1]
 
also you need to run [2] tests.
 
[1]  http://apache-ignite-developers.2346864.n4.nabble.com/Phani-Introduction-td47788.html
[2] https://mtcga.gridgain.com 
>Hello,
>
>Look at the ticket and the only comment I can see is creating a branch on
>git in the main repo and not in my fork. I do not have the right to create a
>branch in the main repository. Am i missing something?
>
>Sorry I probably misread the document but I though that I should fork the
>repo and then pull request as I do not have the rights to create a branch.
>
>Thanks for your help
>
>
>
>--
>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/ 
 
 
 
 

Re: IGNITE-6499 Compact NULL fields

Posted by "steve.hostettler@gmail.com" <st...@gmail.com>.
Hello,

Look at the ticket and the only comment I can see is  creating a branch on
git in the main repo and not in my fork. I do not have the right to create a
branch in the main repository. Am i missing something?

Sorry I probably misread the document but I though that I should fork the
repo and then pull request as I do not have the rights to create a branch.

Thanks for your help



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: IGNITE-6499 Compact NULL fields

Posted by Zhenya Stanilovsky <ar...@mail.ru.INVALID>.
Steve i place some comments in ticket, still have no response.

 
>
>
>------- Forwarded message -------
>From: " steve.hostettler@gmail.com " < steve.hostettler@gmail.com >
>To:  dev@ignite.apache.org
>Cc:
>Subject: Re: Re[4]: IGNITE-6499 Compact NULL fields
>Date: Fri, 12 Jun 2020 16:15:37 +0300
>
>Hello,
>
>Stanilovsky Evgeny : would you mind having a look at the pull request.
>
>Thanks in advance
>
>
>
>--
>Sent from:  http://apache-ignite-developers.2346864.n4.nabble.com/ 
 
 
 
 

Re: Re[4]: IGNITE-6499 Compact NULL fields

Posted by "steve.hostettler@gmail.com" <st...@gmail.com>.
Hello,

Stanilovsky Evgeny : would you mind having a look at the pull request. 

Thanks in advance



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: Re[4]: IGNITE-6499 Compact NULL fields

Posted by "steve.hostettler@gmail.com" <st...@gmail.com>.
Hello,

still fighting to get my unit tests green. Something I do not really
understand is the mechanism of object replacement. For instance, marshalling
a LocalDateTime, gets replace by java.time.Ser, I assume that's because it
has a method writeReplace. But that takes precedence over a custom
serialiser explicitly declared for LocalDateTime. This is quite confusing to
me. Is there some piece of documentation (other than the code) that I can
look at. At this point, I feel like I am doing modification blindly.



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: Re[4]: IGNITE-6499 Compact NULL fields

Posted by "steve.hostettler@gmail.com" <st...@gmail.com>.
Ok, anyway I will finish my patch 2 unit tests still not working. I assume
anyway that if we would want to apply zstd compression we would reuse the
existing page compression algorithm in memory and not only for persistence.
That would probably be anyway simpler and more straightforward.

Will try to hurry to submit a patch for review.



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/