You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2020/01/22 19:36:36 UTC
Old geode-benchmark PRs
Hi,
I noticed we have some old outstanding PRs for the geode-benchmarks
project. Are any of these things we want to merge or should we close them
out?
https://github.com/apache/geode-benchmarks/pulls
-Dan
Re: Old geode-benchmark PRs
Posted by Jacob Barrett <jb...@pivotal.io>.
Why not just leave the PR open then?
> On Jan 23, 2020, at 9:52 AM, Robert Houghton <rh...@pivotal.io> wrote:
>
> @Alberto I like this idea. The link between Jira and the GitHub PR will be
> a nice breadcrumb to follow going forward as well, assuming the PRs are
> correctly labeled for cross-linking to occur.
>
>
> On Thu, Jan 23, 2020 at 8:05 AM Alberto Bustamante Reyes
> <al...@est.tech> wrote:
>
>> What about closing the PRs and creating a Jira ticket (or tickets) for the
>> review and update of the code?
>> If someone finds time to spend on benchmarks, at least he/she will find
>> the tickets in Jira.
>>
>>
>> ________________________________
>> De: Donal Evans <do...@pivotal.io>
>> Enviado: jueves, 23 de enero de 2020 16:45
>> Para: dev@geode.apache.org <de...@geode.apache.org>
>> Asunto: Re: Old geode-benchmark PRs
>>
>> @Alexander, I haven't looked at them in months and they never received any
>> formal review on GitHub, so it's hard to know for sure if they're ready to
>> merge or not, but as Jake said, they probably need some massaging to get
>> the resource usage just right and minimize variance. If at this point
>> there's no-one who knows enough about tuning benchmarks with the time to
>> look at them, then it seems unlikely that they'll get merged any time soon.
>>
>> On Thu, Jan 23, 2020 at 6:42 AM Alexander Murmann <am...@apache.org>
>> wrote:
>>
>>> Donal, are you still looking at these? If they aren't ready to merge and
>>> not being worked on, should they be closed?
>>>
>>> On Wed, Jan 22, 2020 at 3:32 PM Donal Evans <do...@pivotal.io> wrote:
>>>
>>>> Two of those PRs are mine, so perhaps I can give a bit of context for
>>>> people who might look at them. The oldest of the two, "Feature/Add
>>> PdxType
>>>> benchmark and additional framework flexibility" was an attempt to
>>> quantify
>>>> and maintain the improvement in performance for PdxType creation when
>>> large
>>>> numbers of PdxTypes already exist, and to allow the passing of
>> additional
>>>> system properties to the VMs hosting the servers in order to change the
>>> log
>>>> level and prevent the benchmark measuring how long it takes to log
>>> PdxType
>>>> creation rather than actual time taken to create new PdxTypes. This PR
>>> has
>>>> been open for a very long time, so it's possible that the changes
>>> regarding
>>>> passing additional system properties to the VMs are now outdated or
>>>> unnecessary, but the actual benchmarks themselves still have some
>> value.
>>>>
>>>> The second PR, "Added benchmarks for aggregate functions" contains 16
>> new
>>>> benchmarks related to aggregate OQL queries, (8 each for Partitioned
>> and
>>>> Replicated regions), which were added following work in that area by
>> the
>>>> Commons team. The build is currently marked as failing, but this is due
>>> to
>>>> a timeout rather than an actual build failure, as the number of
>>> benchmarks
>>>> added increased the total time to build beyond the currently configured
>>>> timeout. Adding such a large number of additional benchmarks will
>>> probably
>>>> also noticeably increase the time it takes benchmarks to run, which
>> bears
>>>> consideration.
>>>>
>>>> I hope this helps shed some light for people who may look over those
>> PRs.
>>>>
>>>> On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I noticed we have some old outstanding PRs for the geode-benchmarks
>>>>> project. Are any of these things we want to merge or should we close
>>> them
>>>>> out?
>>>>>
>>>>> https://github.com/apache/geode-benchmarks/pulls
>>>>>
>>>>> -Dan
>>>>>
>>>>
>>>
>>
Re: Old geode-benchmark PRs
Posted by Robert Houghton <rh...@pivotal.io>.
@Alberto I like this idea. The link between Jira and the GitHub PR will be
a nice breadcrumb to follow going forward as well, assuming the PRs are
correctly labeled for cross-linking to occur.
On Thu, Jan 23, 2020 at 8:05 AM Alberto Bustamante Reyes
<al...@est.tech> wrote:
> What about closing the PRs and creating a Jira ticket (or tickets) for the
> review and update of the code?
> If someone finds time to spend on benchmarks, at least he/she will find
> the tickets in Jira.
>
>
> ________________________________
> De: Donal Evans <do...@pivotal.io>
> Enviado: jueves, 23 de enero de 2020 16:45
> Para: dev@geode.apache.org <de...@geode.apache.org>
> Asunto: Re: Old geode-benchmark PRs
>
> @Alexander, I haven't looked at them in months and they never received any
> formal review on GitHub, so it's hard to know for sure if they're ready to
> merge or not, but as Jake said, they probably need some massaging to get
> the resource usage just right and minimize variance. If at this point
> there's no-one who knows enough about tuning benchmarks with the time to
> look at them, then it seems unlikely that they'll get merged any time soon.
>
> On Thu, Jan 23, 2020 at 6:42 AM Alexander Murmann <am...@apache.org>
> wrote:
>
> > Donal, are you still looking at these? If they aren't ready to merge and
> > not being worked on, should they be closed?
> >
> > On Wed, Jan 22, 2020 at 3:32 PM Donal Evans <do...@pivotal.io> wrote:
> >
> > > Two of those PRs are mine, so perhaps I can give a bit of context for
> > > people who might look at them. The oldest of the two, "Feature/Add
> > PdxType
> > > benchmark and additional framework flexibility" was an attempt to
> > quantify
> > > and maintain the improvement in performance for PdxType creation when
> > large
> > > numbers of PdxTypes already exist, and to allow the passing of
> additional
> > > system properties to the VMs hosting the servers in order to change the
> > log
> > > level and prevent the benchmark measuring how long it takes to log
> > PdxType
> > > creation rather than actual time taken to create new PdxTypes. This PR
> > has
> > > been open for a very long time, so it's possible that the changes
> > regarding
> > > passing additional system properties to the VMs are now outdated or
> > > unnecessary, but the actual benchmarks themselves still have some
> value.
> > >
> > > The second PR, "Added benchmarks for aggregate functions" contains 16
> new
> > > benchmarks related to aggregate OQL queries, (8 each for Partitioned
> and
> > > Replicated regions), which were added following work in that area by
> the
> > > Commons team. The build is currently marked as failing, but this is due
> > to
> > > a timeout rather than an actual build failure, as the number of
> > benchmarks
> > > added increased the total time to build beyond the currently configured
> > > timeout. Adding such a large number of additional benchmarks will
> > probably
> > > also noticeably increase the time it takes benchmarks to run, which
> bears
> > > consideration.
> > >
> > > I hope this helps shed some light for people who may look over those
> PRs.
> > >
> > > On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
> > >
> > > > Hi,
> > > >
> > > > I noticed we have some old outstanding PRs for the geode-benchmarks
> > > > project. Are any of these things we want to merge or should we close
> > them
> > > > out?
> > > >
> > > > https://github.com/apache/geode-benchmarks/pulls
> > > >
> > > > -Dan
> > > >
> > >
> >
>
RE: Old geode-benchmark PRs
Posted by Alberto Bustamante Reyes <al...@est.tech>.
What about closing the PRs and creating a Jira ticket (or tickets) for the review and update of the code?
If someone finds time to spend on benchmarks, at least he/she will find the tickets in Jira.
________________________________
De: Donal Evans <do...@pivotal.io>
Enviado: jueves, 23 de enero de 2020 16:45
Para: dev@geode.apache.org <de...@geode.apache.org>
Asunto: Re: Old geode-benchmark PRs
@Alexander, I haven't looked at them in months and they never received any
formal review on GitHub, so it's hard to know for sure if they're ready to
merge or not, but as Jake said, they probably need some massaging to get
the resource usage just right and minimize variance. If at this point
there's no-one who knows enough about tuning benchmarks with the time to
look at them, then it seems unlikely that they'll get merged any time soon.
On Thu, Jan 23, 2020 at 6:42 AM Alexander Murmann <am...@apache.org>
wrote:
> Donal, are you still looking at these? If they aren't ready to merge and
> not being worked on, should they be closed?
>
> On Wed, Jan 22, 2020 at 3:32 PM Donal Evans <do...@pivotal.io> wrote:
>
> > Two of those PRs are mine, so perhaps I can give a bit of context for
> > people who might look at them. The oldest of the two, "Feature/Add
> PdxType
> > benchmark and additional framework flexibility" was an attempt to
> quantify
> > and maintain the improvement in performance for PdxType creation when
> large
> > numbers of PdxTypes already exist, and to allow the passing of additional
> > system properties to the VMs hosting the servers in order to change the
> log
> > level and prevent the benchmark measuring how long it takes to log
> PdxType
> > creation rather than actual time taken to create new PdxTypes. This PR
> has
> > been open for a very long time, so it's possible that the changes
> regarding
> > passing additional system properties to the VMs are now outdated or
> > unnecessary, but the actual benchmarks themselves still have some value.
> >
> > The second PR, "Added benchmarks for aggregate functions" contains 16 new
> > benchmarks related to aggregate OQL queries, (8 each for Partitioned and
> > Replicated regions), which were added following work in that area by the
> > Commons team. The build is currently marked as failing, but this is due
> to
> > a timeout rather than an actual build failure, as the number of
> benchmarks
> > added increased the total time to build beyond the currently configured
> > timeout. Adding such a large number of additional benchmarks will
> probably
> > also noticeably increase the time it takes benchmarks to run, which bears
> > consideration.
> >
> > I hope this helps shed some light for people who may look over those PRs.
> >
> > On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
> >
> > > Hi,
> > >
> > > I noticed we have some old outstanding PRs for the geode-benchmarks
> > > project. Are any of these things we want to merge or should we close
> them
> > > out?
> > >
> > > https://github.com/apache/geode-benchmarks/pulls
> > >
> > > -Dan
> > >
> >
>
Re: Old geode-benchmark PRs
Posted by Donal Evans <do...@pivotal.io>.
@Alexander, I haven't looked at them in months and they never received any
formal review on GitHub, so it's hard to know for sure if they're ready to
merge or not, but as Jake said, they probably need some massaging to get
the resource usage just right and minimize variance. If at this point
there's no-one who knows enough about tuning benchmarks with the time to
look at them, then it seems unlikely that they'll get merged any time soon.
On Thu, Jan 23, 2020 at 6:42 AM Alexander Murmann <am...@apache.org>
wrote:
> Donal, are you still looking at these? If they aren't ready to merge and
> not being worked on, should they be closed?
>
> On Wed, Jan 22, 2020 at 3:32 PM Donal Evans <do...@pivotal.io> wrote:
>
> > Two of those PRs are mine, so perhaps I can give a bit of context for
> > people who might look at them. The oldest of the two, "Feature/Add
> PdxType
> > benchmark and additional framework flexibility" was an attempt to
> quantify
> > and maintain the improvement in performance for PdxType creation when
> large
> > numbers of PdxTypes already exist, and to allow the passing of additional
> > system properties to the VMs hosting the servers in order to change the
> log
> > level and prevent the benchmark measuring how long it takes to log
> PdxType
> > creation rather than actual time taken to create new PdxTypes. This PR
> has
> > been open for a very long time, so it's possible that the changes
> regarding
> > passing additional system properties to the VMs are now outdated or
> > unnecessary, but the actual benchmarks themselves still have some value.
> >
> > The second PR, "Added benchmarks for aggregate functions" contains 16 new
> > benchmarks related to aggregate OQL queries, (8 each for Partitioned and
> > Replicated regions), which were added following work in that area by the
> > Commons team. The build is currently marked as failing, but this is due
> to
> > a timeout rather than an actual build failure, as the number of
> benchmarks
> > added increased the total time to build beyond the currently configured
> > timeout. Adding such a large number of additional benchmarks will
> probably
> > also noticeably increase the time it takes benchmarks to run, which bears
> > consideration.
> >
> > I hope this helps shed some light for people who may look over those PRs.
> >
> > On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
> >
> > > Hi,
> > >
> > > I noticed we have some old outstanding PRs for the geode-benchmarks
> > > project. Are any of these things we want to merge or should we close
> them
> > > out?
> > >
> > > https://github.com/apache/geode-benchmarks/pulls
> > >
> > > -Dan
> > >
> >
>
Re: Old geode-benchmark PRs
Posted by Alexander Murmann <am...@apache.org>.
Donal, are you still looking at these? If they aren't ready to merge and
not being worked on, should they be closed?
On Wed, Jan 22, 2020 at 3:32 PM Donal Evans <do...@pivotal.io> wrote:
> Two of those PRs are mine, so perhaps I can give a bit of context for
> people who might look at them. The oldest of the two, "Feature/Add PdxType
> benchmark and additional framework flexibility" was an attempt to quantify
> and maintain the improvement in performance for PdxType creation when large
> numbers of PdxTypes already exist, and to allow the passing of additional
> system properties to the VMs hosting the servers in order to change the log
> level and prevent the benchmark measuring how long it takes to log PdxType
> creation rather than actual time taken to create new PdxTypes. This PR has
> been open for a very long time, so it's possible that the changes regarding
> passing additional system properties to the VMs are now outdated or
> unnecessary, but the actual benchmarks themselves still have some value.
>
> The second PR, "Added benchmarks for aggregate functions" contains 16 new
> benchmarks related to aggregate OQL queries, (8 each for Partitioned and
> Replicated regions), which were added following work in that area by the
> Commons team. The build is currently marked as failing, but this is due to
> a timeout rather than an actual build failure, as the number of benchmarks
> added increased the total time to build beyond the currently configured
> timeout. Adding such a large number of additional benchmarks will probably
> also noticeably increase the time it takes benchmarks to run, which bears
> consideration.
>
> I hope this helps shed some light for people who may look over those PRs.
>
> On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
>
> > Hi,
> >
> > I noticed we have some old outstanding PRs for the geode-benchmarks
> > project. Are any of these things we want to merge or should we close them
> > out?
> >
> > https://github.com/apache/geode-benchmarks/pulls
> >
> > -Dan
> >
>
Re: Old geode-benchmark PRs
Posted by Jacob Barrett <jb...@pivotal.io>.
I hate to just close what’s there but it also likely needs some massaging. Additions to the benchmarks risk destabilizing the CI if the benchmark has a wide variance. I don’t have time right now to spend on tuning new benchmarks. If someone else does please step up and take it on.
> On Jan 22, 2020, at 12:31 PM, Donal Evans <do...@pivotal.io> wrote:
>
> Two of those PRs are mine, so perhaps I can give a bit of context for
> people who might look at them. The oldest of the two, "Feature/Add PdxType
> benchmark and additional framework flexibility" was an attempt to quantify
> and maintain the improvement in performance for PdxType creation when large
> numbers of PdxTypes already exist, and to allow the passing of additional
> system properties to the VMs hosting the servers in order to change the log
> level and prevent the benchmark measuring how long it takes to log PdxType
> creation rather than actual time taken to create new PdxTypes. This PR has
> been open for a very long time, so it's possible that the changes regarding
> passing additional system properties to the VMs are now outdated or
> unnecessary, but the actual benchmarks themselves still have some value.
>
> The second PR, "Added benchmarks for aggregate functions" contains 16 new
> benchmarks related to aggregate OQL queries, (8 each for Partitioned and
> Replicated regions), which were added following work in that area by the
> Commons team. The build is currently marked as failing, but this is due to
> a timeout rather than an actual build failure, as the number of benchmarks
> added increased the total time to build beyond the currently configured
> timeout. Adding such a large number of additional benchmarks will probably
> also noticeably increase the time it takes benchmarks to run, which bears
> consideration.
>
> I hope this helps shed some light for people who may look over those PRs.
>
> On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
>
>> Hi,
>>
>> I noticed we have some old outstanding PRs for the geode-benchmarks
>> project. Are any of these things we want to merge or should we close them
>> out?
>>
>> https://github.com/apache/geode-benchmarks/pulls
>>
>> -Dan
>>
Re: Old geode-benchmark PRs
Posted by Donal Evans <do...@pivotal.io>.
Two of those PRs are mine, so perhaps I can give a bit of context for
people who might look at them. The oldest of the two, "Feature/Add PdxType
benchmark and additional framework flexibility" was an attempt to quantify
and maintain the improvement in performance for PdxType creation when large
numbers of PdxTypes already exist, and to allow the passing of additional
system properties to the VMs hosting the servers in order to change the log
level and prevent the benchmark measuring how long it takes to log PdxType
creation rather than actual time taken to create new PdxTypes. This PR has
been open for a very long time, so it's possible that the changes regarding
passing additional system properties to the VMs are now outdated or
unnecessary, but the actual benchmarks themselves still have some value.
The second PR, "Added benchmarks for aggregate functions" contains 16 new
benchmarks related to aggregate OQL queries, (8 each for Partitioned and
Replicated regions), which were added following work in that area by the
Commons team. The build is currently marked as failing, but this is due to
a timeout rather than an actual build failure, as the number of benchmarks
added increased the total time to build beyond the currently configured
timeout. Adding such a large number of additional benchmarks will probably
also noticeably increase the time it takes benchmarks to run, which bears
consideration.
I hope this helps shed some light for people who may look over those PRs.
On Wed, Jan 22, 2020 at 11:36 AM Dan Smith <ds...@pivotal.io> wrote:
> Hi,
>
> I noticed we have some old outstanding PRs for the geode-benchmarks
> project. Are any of these things we want to merge or should we close them
> out?
>
> https://github.com/apache/geode-benchmarks/pulls
>
> -Dan
>