Re: [PROPOSAL] Change the default value of conserve-sockets to false
I'm all in for changing the default to *false* but, unfortunately and for all the reasons already stated in the thread, I'm hesitant to include this change as part of a minor release. Best regards. On Thu, 19 Nov 2020 at 02:48, John Blum wrote: > The downside of conserve-sockets = false is that you are (essentially) > back to a Thread|Socket / Request model (though Geode limits this system > resource consumption to a degree by the use of Thread Pools in p2p > distribution layer) and thus, you can run out of file descriptors (per > newly opened Socket) pretty quickly if you are not careful. > > conserve-sockets set to true limits the use of finite system resources why > risking deadlocks (i.e. A -> B -> C -> A), which is also contingent on ACKS > (and the infamous ReplyProcessor21; at least at 1 time, not sure if it is > still in play, but probably!). > > conserve-sockets set to false uses more system resources but avoids > deadlocks. > > If this change is made, I'd minimally make sure to document that users > should adjust their (soft & hard) ulimits accordingly, based on use cases, > load, etc. > > Personally, this has caused enough grief in the past (both ways, > actually!) that I 'd say this is a major version change. > > -j > > > > From: Nabarun Nag > Sent: Wednesday, November 18, 2020 6:09 PM > To: dev@geode.apache.org > Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to > false > > +1 > > * As nearly all of the production use-cases need "conserve-sockets" to > be set to false, I think we can aim for changing the default value to false > for 1.14.0 release. > * We can highlight this change in the release notes and emails. > > Regards, > Nabarun > > > From: Udo Kohlmeyer > Sent: Wednesday, November 18, 2020 6:00 PM > To: dev@geode.apache.org > Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to > false > > Hi there Donal, > > Thank you for raising this. It is not an uncommon request to change the > default value of this field. > > This has been discussed many times in the past. I would LOVE to approve > this change, but this would mean that users that don’t set this property > might suddenly have this property changed. We are not sure that this would > mean for these users. BUT > > That said, there have been very little (to none) complaints about the > product stability when `conserve-sockets=false` are set. > > +1 - if we are allowed to make this change outside of a major version > change. > > --Udo > > From: Donal Evans > Date: Thursday, November 19, 2020 at 12:04 PM > To: dev@geode.apache.org > Subject: [PROPOSAL] Change the default value of conserve-sockets to false > Hi Geode dev, > > First, from the docs[1], a brief explanation of the purpose of the > conserve-sockets property: > > "The conserve-sockets setting indicates whether application threads share > sockets with other threads or use their own sockets for member > communication. This setting has no effect on communication between a server > and its clients, but it does control the server’s communication with its > peers or a gateway sender’s communication with a gateway receiver." > > The current default value for the conserve-sockets property is true, which > at first glance makes sense, since in an ideal world, existing sockets > could be shared between threads and there would be no need to create and > destroy new sockets for each process, which can be somewhat > resource-intensive. However, in practice, there are several known issues > with using the default setting of true. From the docs[1]: > > "For distributed regions, the put operation, and destroy and invalidate > for regions and entries, can all be optimized with conserve-sockets set to > false. For partitioned regions, setting conserve-sockets to false can > improve general throughput. > Note: When you have transactions operating on EMPTY, NORMAL or PARTITION > regions, make sure that conserve-sockets is set to false to avoid > distributed deadlocks." > > and[2]: > > "WAN deployments increase the messaging demands on a Geode system. To > avoid hangs related to WAN messaging, always set `conserve-sockets=false` > for Geode members that participate in a WAN deployment." > > Given that it is generally accepted as best practice to set > conserve-sockets to false for almost all use cases of Geode beyond the most > simple, it would make sense to also change the default value to false, to > prevent people having to encounter a problem, search for the solution, then > change the setting to what is almost always the "correct" value. > > I have done some experimenting to see what it would take to make this > proposal a reality, and the changes required are very minimal, with only > two existing DUnit tests that need to be modified to explicitly set the > value of conserve-sockets that were previously relying on the default being > true. > > Any feedback on this propos
apache-geode-1.13.0.tgz not found in LGTM analysis
Hi, I am getting the following error in the LGTM analysis of some pull requests since yesterday (for example https://github.com/apache/geode-native/pull/690): [2020-11-19 07:25:41] [build-err] + wget -O apache-geode.tgz http://mirror.transip.net/apache/geode/1.13.0/apache-geode-1.13.0.tgz [2020-11-19 07:25:41] [build-err] --2020-11-19 07:25:41-- http://mirror.transip.net/apache/geode/1.13.0/apache-geode-1.13.0.tgz [2020-11-19 07:25:41] [build-err] Resolving mirror.transip.net (mirror.transip.net)... 149.210.210.109, 2a01:7c8:1337::100 [2020-11-19 07:25:41] [build-err] Connecting to mirror.transip.net (mirror.transip.net)|149.210.210.109|:80... connected. [2020-11-19 07:25:41] [build-err] HTTP request sent, awaiting response... 404 Not Found It seems the tgz file is not available anymore. Any idea how to fix it? Thanks, /Alberto G.
Re: apache-geode-1.13.0.tgz not found in LGTM analysis
It looks like it was hardcoded[1] that way recently. Geode 1.13.1 was just announced[2] so you are correct, 1.13.0 is archived and no longer on the mirrors. If maintaining a hardcoded Geode version number in geode-native is necessary, the set_versions[3] script should be updated to keep it in sync. [1] https://github.com/apache/geode-native/blame/develop/.lgtm.yml [2] https://lists.apache.org/x/thread.html/rf937beb3783dc7f2e27a2618586d8cacd8b231793cccab863f4632e3@%3Cdev.geode.apache.org%3E [3] https://github.com/apache/geode/blob/develop/dev-tools/release/set_versions.sh -Owen From: Alberto Gomez Date: Thursday, November 19, 2020 at 2:39 AM To: dev@geode.apache.org Subject: apache-geode-1.13.0.tgz not found in LGTM analysis Hi, I am getting the following error in the LGTM analysis of some pull requests since yesterday (for example https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F690&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=HY8tdLeIfM20hv7PLX%2B%2BYcStTD%2Fq334X7UXF6umRVL8%3D&reserved=0): [2020-11-19 07:25:41] [build-err] + wget -O apache-geode.tgz https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=mGRuhIujL%2F%2FRo4EciDy3sC7uLwJJiR7UYDTMEJGDf%2BA%3D&reserved=0 [2020-11-19 07:25:41] [build-err] --2020-11-19 07:25:41-- https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=mGRuhIujL%2F%2FRo4EciDy3sC7uLwJJiR7UYDTMEJGDf%2BA%3D&reserved=0 [2020-11-19 07:25:41] [build-err] Resolving mirror.transip.net (mirror.transip.net)... 149.210.210.109, 2a01:7c8:1337::100 [2020-11-19 07:25:41] [build-err] Connecting to mirror.transip.net (mirror.transip.net)|149.210.210.109|:80... connected. [2020-11-19 07:25:41] [build-err] HTTP request sent, awaiting response... 404 Not Found It seems the tgz file is not available anymore. Any idea how to fix it? Thanks, /Alberto G.
Re: apache-geode-1.13.0.tgz not found in LGTM analysis
Thanks for the info, Owen. I have created a JIRA and a PR to update the .lgtm.yml file in the geode-native repo: https://github.com/apache/geode-native/pull/698 Any volunteer to review it? BR, Alberto From: Owen Nichols Sent: Thursday, November 19, 2020 11:50 AM To: dev@geode.apache.org Subject: Re: apache-geode-1.13.0.tgz not found in LGTM analysis It looks like it was hardcoded[1] that way recently. Geode 1.13.1 was just announced[2] so you are correct, 1.13.0 is archived and no longer on the mirrors. If maintaining a hardcoded Geode version number in geode-native is necessary, the set_versions[3] script should be updated to keep it in sync. [1] https://github.com/apache/geode-native/blame/develop/.lgtm.yml [2] https://lists.apache.org/x/thread.html/rf937beb3783dc7f2e27a2618586d8cacd8b231793cccab863f4632e3@%3Cdev.geode.apache.org%3E [3] https://github.com/apache/geode/blob/develop/dev-tools/release/set_versions.sh -Owen From: Alberto Gomez Date: Thursday, November 19, 2020 at 2:39 AM To: dev@geode.apache.org Subject: apache-geode-1.13.0.tgz not found in LGTM analysis Hi, I am getting the following error in the LGTM analysis of some pull requests since yesterday (for example https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F690&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=HY8tdLeIfM20hv7PLX%2B%2BYcStTD%2Fq334X7UXF6umRVL8%3D&reserved=0): [2020-11-19 07:25:41] [build-err] + wget -O apache-geode.tgz https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=mGRuhIujL%2F%2FRo4EciDy3sC7uLwJJiR7UYDTMEJGDf%2BA%3D&reserved=0 [2020-11-19 07:25:41] [build-err] --2020-11-19 07:25:41-- https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C67b7059e6f8945508ddc08d88c7751f0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413791402422145%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=mGRuhIujL%2F%2FRo4EciDy3sC7uLwJJiR7UYDTMEJGDf%2BA%3D&reserved=0 [2020-11-19 07:25:41] [build-err] Resolving mirror.transip.net (mirror.transip.net)... 149.210.210.109, 2a01:7c8:1337::100 [2020-11-19 07:25:41] [build-err] Connecting to mirror.transip.net (mirror.transip.net)|149.210.210.109|:80... connected. [2020-11-19 07:25:41] [build-err] HTTP request sent, awaiting response... 404 Not Found It seems the tgz file is not available anymore. Any idea how to fix it? Thanks, /Alberto G.
Re: apache-geode-1.13.0.tgz not found in LGTM analysis
One of my biggest beefs is that we have to hard code the current version all over the place. It's in the docker image used by native to run Travis jobs. It’s also in the benchmark job Gradle and shell scripts. Feels like there has got to be a better way. Ideally the develop branches of these other repos should be pulling the last good snapshot version and not the last release. If the last release is needed there has to be a way to get that version from the Apache mirror process. -Jake > On Nov 19, 2020, at 3:43 AM, Alberto Gomez wrote: > > Thanks for the info, Owen. > > I have created a JIRA and a PR to update the .lgtm.yml file in the > geode-native repo: > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F698&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180811687%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9%2BBT3zGFmAAtCxJRKcDaXt3YzTAGtVtfglr0IYo%2F7xA%3D&reserved=0 > > Any volunteer to review it? > > BR, > > Alberto > > From: Owen Nichols > Sent: Thursday, November 19, 2020 11:50 AM > To: dev@geode.apache.org > Subject: Re: apache-geode-1.13.0.tgz not found in LGTM analysis > > It looks like it was hardcoded[1] that way recently. Geode 1.13.1 was just > announced[2] so you are correct, 1.13.0 is archived and no longer on the > mirrors. > > If maintaining a hardcoded Geode version number in geode-native is necessary, > the set_versions[3] script should be updated to keep it in sync. > > [1] > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fblame%2Fdevelop%2F.lgtm.yml&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180811687%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=sx%2B3VPvgaKExbozRhsgRevap%2ForjxMfMz92daXIknBk%3D&reserved=0 > [2] > https://lists.apache.org/x/thread.html/rf937beb3783dc7f2e27a2618586d8cacd8b231793cccab863f4632e3@%3Cdev.geode.apache.org%3E > [3] > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fdev-tools%2Frelease%2Fset_versions.sh&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180821685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=sdkrae2x2NP7TPhNY0wIEJ%2B2%2BwJWEqVXbX0H4WgepIk%3D&reserved=0 > > -Owen > > From: Alberto Gomez > Date: Thursday, November 19, 2020 at 2:39 AM > To: dev@geode.apache.org > Subject: apache-geode-1.13.0.tgz not found in LGTM analysis > Hi, > > I am getting the following error in the LGTM analysis of some pull requests > since yesterday (for example > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F690&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180821685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=IbxXYHoYLEq7PQOE51y%2FQyjUdetMj%2BM2z%2FUfq7%2B77yk%3D&reserved=0): > > [2020-11-19 07:25:41] [build-err] + wget -O apache-geode.tgz > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180821685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=HNKNarx0%2F4XOqmbobWSFc18duxI6OvN1IkJzNrw5So8%3D&reserved=0 > [2020-11-19 07:25:41] [build-err] --2020-11-19 07:25:41-- > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Cjabarrett%40vmware.com%7C5a37f006a509463481a908d88c8059c6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637413830180821685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=HNKNarx0%2F4XOqmbobWSFc18duxI6OvN1IkJzNrw5So8%3D&reserved=0 > [2020-11-19 07:25:41] [build-err] Resolving mirror.transip.net > (mirror.transip.net)... 149.210.210.109, 2a01:7c8:1337::100 > [2020-11-19 07:25:41] [build-err] Connecting to mirror.transip.net > (mirror.transip.net)|149.210.210.109|:80... connected. > [2020-11-19 07:25:41] [build-err] HTTP request sent, awaiting response... 404 > Not Found > > It seems the tgz file is not available anymore. > > Any idea how to fix it? > > Thanks, > > /Alberto G.
Re: [PROPOSAL] Change the default value of conserve-sockets to false
I would argue that is doesn’t change the outward behavior of the product. It does change the internal workings of the product. It does improve the performance and reliability. As long as changes to the internals don’t have effect on the outward facing behavior of the product I see no problem making these internal changes. Yes this may lead to more resource utilization but so can and have so other changes along the way between majors. I would also expect in the scenarios where this would make the most impact on resources they are already configured to use this feature. +1 make the change. > On Nov 19, 2020, at 1:16 AM, Ju@N wrote: > > I'm all in for changing the default to *false* but, unfortunately and for > all the reasons already stated in the thread, I'm hesitant to include this > change as part of a minor release. > Best regards. > > On Thu, 19 Nov 2020 at 02:48, John Blum wrote: > >> The downside of conserve-sockets = false is that you are (essentially) >> back to a Thread|Socket / Request model (though Geode limits this system >> resource consumption to a degree by the use of Thread Pools in p2p >> distribution layer) and thus, you can run out of file descriptors (per >> newly opened Socket) pretty quickly if you are not careful. >> >> conserve-sockets set to true limits the use of finite system resources why >> risking deadlocks (i.e. A -> B -> C -> A), which is also contingent on ACKS >> (and the infamous ReplyProcessor21; at least at 1 time, not sure if it is >> still in play, but probably!). >> >> conserve-sockets set to false uses more system resources but avoids >> deadlocks. >> >> If this change is made, I'd minimally make sure to document that users >> should adjust their (soft & hard) ulimits accordingly, based on use cases, >> load, etc. >> >> Personally, this has caused enough grief in the past (both ways, >> actually!) that I 'd say this is a major version change. >> >> -j >> >> >> >> From: Nabarun Nag >> Sent: Wednesday, November 18, 2020 6:09 PM >> To: dev@geode.apache.org >> Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to >> false >> >> +1 >> >> * As nearly all of the production use-cases need "conserve-sockets" to >> be set to false, I think we can aim for changing the default value to false >> for 1.14.0 release. >> * We can highlight this change in the release notes and emails. >> >> Regards, >> Nabarun >> >> >> From: Udo Kohlmeyer >> Sent: Wednesday, November 18, 2020 6:00 PM >> To: dev@geode.apache.org >> Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to >> false >> >> Hi there Donal, >> >> Thank you for raising this. It is not an uncommon request to change the >> default value of this field. >> >> This has been discussed many times in the past. I would LOVE to approve >> this change, but this would mean that users that don’t set this property >> might suddenly have this property changed. We are not sure that this would >> mean for these users. BUT >> >> That said, there have been very little (to none) complaints about the >> product stability when `conserve-sockets=false` are set. >> >> +1 - if we are allowed to make this change outside of a major version >> change. >> >> --Udo >> >> From: Donal Evans >> Date: Thursday, November 19, 2020 at 12:04 PM >> To: dev@geode.apache.org >> Subject: [PROPOSAL] Change the default value of conserve-sockets to false >> Hi Geode dev, >> >> First, from the docs[1], a brief explanation of the purpose of the >> conserve-sockets property: >> >> "The conserve-sockets setting indicates whether application threads share >> sockets with other threads or use their own sockets for member >> communication. This setting has no effect on communication between a server >> and its clients, but it does control the server’s communication with its >> peers or a gateway sender’s communication with a gateway receiver." >> >> The current default value for the conserve-sockets property is true, which >> at first glance makes sense, since in an ideal world, existing sockets >> could be shared between threads and there would be no need to create and >> destroy new sockets for each process, which can be somewhat >> resource-intensive. However, in practice, there are several known issues >> with using the default setting of true. From the docs[1]: >> >> "For distributed regions, the put operation, and destroy and invalidate >> for regions and entries, can all be optimized with conserve-sockets set to >> false. For partitioned regions, setting conserve-sockets to false can >> improve general throughput. >> Note: When you have transactions operating on EMPTY, NORMAL or PARTITION >> regions, make sure that conserve-sockets is set to false to avoid >> distributed deadlocks." >> >> and[2]: >> >> "WAN deployments increase the messaging demands on a Geode system. To >> avoid hangs related to WAN messaging, always set
Re: [PROPOSAL] Change the default value of conserve-sockets to false
Just to clarify a comment from Owen, conserve-sockets=true does show improved performance over conserve-sockets=false in certain specific scenarios, but this isn't a hard and fast rule that applies everywhere, even excluding the cases where using conserve-sockets=true can lead to distributed deadlocks. As an example, with the new default setting of false, the UpgradeTest job in the CI pipeline takes about 10% longer, indicating that, at least for the scenarios being tested there, there is some performance impact. That being said, all of the geode-benchmarks tests explicitly set conserve-sockets=false, so from the point of view of what we're actually testing in terms of performance, no impact will be seen due to this change. From: Jacob Barrett Sent: Thursday, November 19, 2020 8:02 AM To: dev@geode.apache.org Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to false I would argue that is doesn’t change the outward behavior of the product. It does change the internal workings of the product. It does improve the performance and reliability. As long as changes to the internals don’t have effect on the outward facing behavior of the product I see no problem making these internal changes. Yes this may lead to more resource utilization but so can and have so other changes along the way between majors. I would also expect in the scenarios where this would make the most impact on resources they are already configured to use this feature. +1 make the change. > On Nov 19, 2020, at 1:16 AM, Ju@N wrote: > > I'm all in for changing the default to *false* but, unfortunately and for > all the reasons already stated in the thread, I'm hesitant to include this > change as part of a minor release. > Best regards. > > On Thu, 19 Nov 2020 at 02:48, John Blum wrote: > >> The downside of conserve-sockets = false is that you are (essentially) >> back to a Thread|Socket / Request model (though Geode limits this system >> resource consumption to a degree by the use of Thread Pools in p2p >> distribution layer) and thus, you can run out of file descriptors (per >> newly opened Socket) pretty quickly if you are not careful. >> >> conserve-sockets set to true limits the use of finite system resources why >> risking deadlocks (i.e. A -> B -> C -> A), which is also contingent on ACKS >> (and the infamous ReplyProcessor21; at least at 1 time, not sure if it is >> still in play, but probably!). >> >> conserve-sockets set to false uses more system resources but avoids >> deadlocks. >> >> If this change is made, I'd minimally make sure to document that users >> should adjust their (soft & hard) ulimits accordingly, based on use cases, >> load, etc. >> >> Personally, this has caused enough grief in the past (both ways, >> actually!) that I 'd say this is a major version change. >> >> -j >> >> >> >> From: Nabarun Nag >> Sent: Wednesday, November 18, 2020 6:09 PM >> To: dev@geode.apache.org >> Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to >> false >> >> +1 >> >> * As nearly all of the production use-cases need "conserve-sockets" to >> be set to false, I think we can aim for changing the default value to false >> for 1.14.0 release. >> * We can highlight this change in the release notes and emails. >> >> Regards, >> Nabarun >> >> >> From: Udo Kohlmeyer >> Sent: Wednesday, November 18, 2020 6:00 PM >> To: dev@geode.apache.org >> Subject: Re: [PROPOSAL] Change the default value of conserve-sockets to >> false >> >> Hi there Donal, >> >> Thank you for raising this. It is not an uncommon request to change the >> default value of this field. >> >> This has been discussed many times in the past. I would LOVE to approve >> this change, but this would mean that users that don’t set this property >> might suddenly have this property changed. We are not sure that this would >> mean for these users. BUT >> >> That said, there have been very little (to none) complaints about the >> product stability when `conserve-sockets=false` are set. >> >> +1 - if we are allowed to make this change outside of a major version >> change. >> >> --Udo >> >> From: Donal Evans >> Date: Thursday, November 19, 2020 at 12:04 PM >> To: dev@geode.apache.org >> Subject: [PROPOSAL] Change the default value of conserve-sockets to false >> Hi Geode dev, >> >> First, from the docs[1], a brief explanation of the purpose of the >> conserve-sockets property: >> >> "The conserve-sockets setting indicates whether application threads share >> sockets with other threads or use their own sockets for member >> communication. This setting has no effect on communication between a server >> and its clients, but it does control the server’s communication with its >> peers or a gateway sender’s communication with a gateway receiver." >> >> The current default value for the conserve-sockets property is true, which >> at
Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
Does GitHub allow us to limit this automated action to non-DRAFT PRs? On 11/18/20, 8:28 PM, "Owen Nichols" wrote: +1 This will greatly improve the experience for contributors. Instead of an intimidating empty list of reviewers when you submit a PR (and no ability to add reviewers, if you’re not a committer), it will be great to already have at least two reviewers automagically assigned. I have a small concern that initially populating this file via a flurry of PRs may result in a lot of merge conflicts with anyone else that volunteers on the same or an adjacent line. Also, since you _must_ be a committer to be a code owner, is a PR even necessary…would directly committing changes to the feature/introduce-codeowners branch be acceptable? If not, who needs to review and who can merge the PRs against the ‘introduce’ branch? What happens if you are the only owner for an area, can you approve your own PR? Even if the goal is two owners per area, does that mean PRs by either owner cannot be merged if the only other owner is on vacation or otherwise unavailable? Can we submit PRs against the ‘introduce’ branch now and they just won’t be merged before Nov 26, or do we all just need to be patient until this review period has concluded? From: Robert Houghton Date: Wednesday, November 18, 2020 at 2:07 PM To: dev@geode.apache.org Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode Hello Devs. I would like to improve the quality of the pull-request reviews we see for critical parts of the Apache Geode project. In discussions with other committers, a (not the) big hurdle to that is getting the right eyes to look at a given PR. To that end, I propose the adoption of GitHub's CODEOWNERS functionality for the Apache Geode code repository. A discussion-document of this issue has been written up by @upthewaterspout. Thanks Dan! https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfile&data=04%7C01%7Cburghardte%40vmware.com%7C45970ac6ae304f69b7c408d88c437e94%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413568802908369%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=YkXv2cmblejVrM6a9k%2B1tCP2V5U0talgqIl4Ekrhe24%3D&reserved=0 I have tested the feature with fellow Geode committers @upthewaterspout and @onichols-pivotal, and found it to meet our expectations. Please review the document, and comment or reply to this thread, by 25 November, so we might start the task of nominating and applying for ownership. -Robert Houghton
Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
@onichols, to answer your questions: * If there is only one owner, and the owner is on vacation, then a PR will be delayed * If you are an owner, and the only other owner is on vacation, then the PR will be delayed * If one is the sole owner, then the owner-review requirement is not enforced (you can’t review your own PR) * Feel free to start PRs or changes to `features/introduce_codeowners` From: Owen Nichols Date: Wednesday, November 18, 2020 at 8:28 PM To: dev@geode.apache.org Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode +1 This will greatly improve the experience for contributors. Instead of an intimidating empty list of reviewers when you submit a PR (and no ability to add reviewers, if you’re not a committer), it will be great to already have at least two reviewers automagically assigned. I have a small concern that initially populating this file via a flurry of PRs may result in a lot of merge conflicts with anyone else that volunteers on the same or an adjacent line. Also, since you _must_ be a committer to be a code owner, is a PR even necessary…would directly committing changes to the feature/introduce-codeowners branch be acceptable? If not, who needs to review and who can merge the PRs against the ‘introduce’ branch? What happens if you are the only owner for an area, can you approve your own PR? Even if the goal is two owners per area, does that mean PRs by either owner cannot be merged if the only other owner is on vacation or otherwise unavailable? Can we submit PRs against the ‘introduce’ branch now and they just won’t be merged before Nov 26, or do we all just need to be patient until this review period has concluded? From: Robert Houghton Date: Wednesday, November 18, 2020 at 2:07 PM To: dev@geode.apache.org Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode Hello Devs. I would like to improve the quality of the pull-request reviews we see for critical parts of the Apache Geode project. In discussions with other committers, a (not the) big hurdle to that is getting the right eyes to look at a given PR. To that end, I propose the adoption of GitHub's CODEOWNERS functionality for the Apache Geode code repository. A discussion-document of this issue has been written up by @upthewaterspout. Thanks Dan! https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfile&data=04%7C01%7Crhoughton%40vmware.com%7C78b888ac11b9497a0ebf08d88c437d63%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413568802236824%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=h7uZXWAqmYrcyry6q5%2BYH4PRrMdSi9egPc3KsD9lPX8%3D&reserved=0 I have tested the feature with fellow Geode committers @upthewaterspout and @onichols-pivotal, and found it to meet our expectations. Please review the document, and comment or reply to this thread, by 25 November, so we might start the task of nominating and applying for ownership. -Robert Houghton
Re: [PROPOSAL] Change the default value of conserve-sockets to false
Personally, this has caused enough grief in the past (both ways, actually!) that I 'd say this is a major version change. I agree with John. Either value of conserve-sockets can crash or hang your system depending on your use case. If this was just a matter of slowing down or speeding up performance, I think we could change it. But users that are impacted won't just see their system slow down. It will crash or hang. Potentially only with production sized workloads. With conserve-sockets=false every thread on the server creates its own sockets to other servers. With N servers that's N sockets per thread. With our default of a max of 800 threads for client connections and a 20 server cluster you are looking at a worst case of 800 * 20 = 16K sending sockets per server, with another 16K receiving sockets and 16K receiving threads. That's before considering function execution threads, WAN receivers, and various other executors we have on the server. Users with too many threads will hit their file descriptor or thread limits. Or they will run out of memory for thread stacks, socket buffers, etc. -Dan
Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
Hi Ernie, DRAFT PRs do not get reviewers by default, but when the draft transitions to ‘ready’, then the owners are requested to review. From: Ernie Burghardt Date: Thursday, November 19, 2020 at 9:56 AM To: dev@geode.apache.org Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode Does GitHub allow us to limit this automated action to non-DRAFT PRs? On 11/18/20, 8:28 PM, "Owen Nichols" wrote: +1 This will greatly improve the experience for contributors. Instead of an intimidating empty list of reviewers when you submit a PR (and no ability to add reviewers, if you’re not a committer), it will be great to already have at least two reviewers automagically assigned. I have a small concern that initially populating this file via a flurry of PRs may result in a lot of merge conflicts with anyone else that volunteers on the same or an adjacent line. Also, since you _must_ be a committer to be a code owner, is a PR even necessary…would directly committing changes to the feature/introduce-codeowners branch be acceptable? If not, who needs to review and who can merge the PRs against the ‘introduce’ branch? What happens if you are the only owner for an area, can you approve your own PR? Even if the goal is two owners per area, does that mean PRs by either owner cannot be merged if the only other owner is on vacation or otherwise unavailable? Can we submit PRs against the ‘introduce’ branch now and they just won’t be merged before Nov 26, or do we all just need to be patient until this review period has concluded? From: Robert Houghton Date: Wednesday, November 18, 2020 at 2:07 PM To: dev@geode.apache.org Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode Hello Devs. I would like to improve the quality of the pull-request reviews we see for critical parts of the Apache Geode project. In discussions with other committers, a (not the) big hurdle to that is getting the right eyes to look at a given PR. To that end, I propose the adoption of GitHub's CODEOWNERS functionality for the Apache Geode code repository. A discussion-document of this issue has been written up by @upthewaterspout. Thanks Dan! https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfile&data=04%7C01%7Crhoughton%40vmware.com%7C53e36a75a03c4910b4f508d88cb4791b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414054040266773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=xXvg59MWDSLaQsC3Ik5ikZ1MFkCKnlpJaoEQ5EgwB4E%3D&reserved=0 I have tested the feature with fellow Geode committers @upthewaterspout and @onichols-pivotal, and found it to meet our expectations. Please review the document, and comment or reply to this thread, by 25 November, so we might start the task of nominating and applying for ownership. -Robert Houghton
Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
Perfect, then let's give this a try. +1 On 11/19/20, 10:45 AM, "Robert Houghton" wrote: Hi Ernie, DRAFT PRs do not get reviewers by default, but when the draft transitions to ‘ready’, then the owners are requested to review. From: Ernie Burghardt Date: Thursday, November 19, 2020 at 9:56 AM To: dev@geode.apache.org Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode Does GitHub allow us to limit this automated action to non-DRAFT PRs? On 11/18/20, 8:28 PM, "Owen Nichols" wrote: +1 This will greatly improve the experience for contributors. Instead of an intimidating empty list of reviewers when you submit a PR (and no ability to add reviewers, if you’re not a committer), it will be great to already have at least two reviewers automagically assigned. I have a small concern that initially populating this file via a flurry of PRs may result in a lot of merge conflicts with anyone else that volunteers on the same or an adjacent line. Also, since you _must_ be a committer to be a code owner, is a PR even necessary…would directly committing changes to the feature/introduce-codeowners branch be acceptable? If not, who needs to review and who can merge the PRs against the ‘introduce’ branch? What happens if you are the only owner for an area, can you approve your own PR? Even if the goal is two owners per area, does that mean PRs by either owner cannot be merged if the only other owner is on vacation or otherwise unavailable? Can we submit PRs against the ‘introduce’ branch now and they just won’t be merged before Nov 26, or do we all just need to be patient until this review period has concluded? From: Robert Houghton Date: Wednesday, November 18, 2020 at 2:07 PM To: dev@geode.apache.org Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode Hello Devs. I would like to improve the quality of the pull-request reviews we see for critical parts of the Apache Geode project. In discussions with other committers, a (not the) big hurdle to that is getting the right eyes to look at a given PR. To that end, I propose the adoption of GitHub's CODEOWNERS functionality for the Apache Geode code repository. A discussion-document of this issue has been written up by @upthewaterspout. Thanks Dan! https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfile&data=04%7C01%7Cburghardte%40vmware.com%7C99af9622ebb74eb109a508d88cbb3965%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414083036254284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=hHc662NXT538UzqD0J%2Ftt8WalEX0Uo1Ff9hmhWEUmtg%3D&reserved=0 I have tested the feature with fellow Geode committers @upthewaterspout and @onichols-pivotal, and found it to meet our expectations. Please review the document, and comment or reply to this thread, by 25 November, so we might start the task of nominating and applying for ownership. -Robert Houghton
Re: apache-geode-1.13.0.tgz not found in LGTM analysis
I will ll automate the maintenance of then hardcoded version references. Thanks for pointing these out, they were not on my radar… From: Jacob Barrett Date: Thursday, November 19, 2020 at 7:56 AM To: dev@geode.apache.org Subject: Re: apache-geode-1.13.0.tgz not found in LGTM analysis One of my biggest beefs is that we have to hard code the current version all over the place. It's in the docker image used by native to run Travis jobs. It’s also in the benchmark job Gradle and shell scripts. Feels like there has got to be a better way. Ideally the develop branches of these other repos should be pulling the last good snapshot version and not the last release. If the last release is needed there has to be a way to get that version from the Apache mirror process. -Jake > On Nov 19, 2020, at 3:43 AM, Alberto Gomez wrote: > > Thanks for the info, Owen. > > I have created a JIRA and a PR to update the .lgtm.yml file in the > geode-native repo: > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F698&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TO7indEghKcO4j1kt3mT27FZ4G92PSp3fo39BOAg5ZY%3D&reserved=0 > > Any volunteer to review it? > > BR, > > Alberto > > From: Owen Nichols > Sent: Thursday, November 19, 2020 11:50 AM > To: dev@geode.apache.org > Subject: Re: apache-geode-1.13.0.tgz not found in LGTM analysis > > It looks like it was hardcoded[1] that way recently. Geode 1.13.1 was just > announced[2] so you are correct, 1.13.0 is archived and no longer on the > mirrors. > > If maintaining a hardcoded Geode version number in geode-native is necessary, > the set_versions[3] script should be updated to keep it in sync. > > [1] > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fblame%2Fdevelop%2F.lgtm.yml&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2FkxxwBXT5HC5sL8HnUZWi10XsuLUcbUZNSBY0zrHQ4Q%3D&reserved=0 > [2] > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.apache.org%2Fx%2Fthread.html%2Frf937beb3783dc7f2e27a2618586d8cacd8b231793cccab863f4632e3%40%253Cdev.geode.apache.org%253E&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=q32%2BF6nslKfPc6igB8PtN%2FK26OUk6kebZOEAlMIRx4I%3D&reserved=0 > [3] > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fdev-tools%2Frelease%2Fset_versions.sh&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=m1e8%2BBSwtojZ4%2F9meaFcR8MMHaywe9FeEoZDkagYJJE%3D&reserved=0 > > -Owen > > From: Alberto Gomez > Date: Thursday, November 19, 2020 at 2:39 AM > To: dev@geode.apache.org > Subject: apache-geode-1.13.0.tgz not found in LGTM analysis > Hi, > > I am getting the following error in the LGTM analysis of some pull requests > since yesterday (for example > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode-native%2Fpull%2F690&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jY8z5pxNJMUbEJQVMNweXhGTLL%2BOSzL7jc5LGgwXNKk%3D&reserved=0): > > [2020-11-19 07:25:41] [build-err] + wget -O apache-geode.tgz > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605633049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=0mLaFpiKbnggmyDwfW%2Fo0XO9EAIqY2WhzeWJuF1dBJU%3D&reserved=0 > [2020-11-19 07:25:41] [build-err] --2020-11-19 07:25:41-- > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmirror.transip.net%2Fapache%2Fgeode%2F1.13.0%2Fapache-geode-1.13.0.tgz&data=04%7C01%7Conichols%40vmware.com%7C6ed7bcf7e83b4bc0924608d88ca39bf4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637413981605643043%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
Re: apache-geode-1.13.0.tgz not found in LGTM analysis
Agreed…although snapshots will never be on Apache release mirrors. We do have alternate locations for builds that go through the CI pipelines we could use. Anthony On Nov 19, 2020, at 7:55 AM, Jacob Barrett mailto:jabarr...@vmware.com>> wrote: Ideally the develop branches of these other repos should be pulling the last good snapshot version and not the last release. If the last release is needed there has to be a way to get that version from the Apache mirror process.
Re: [DISCUSS] Adding CODEOWNERS to Apache Geode
+1 I think we as a project will need to iterator on the code owners as well as the process for code owners. But this is a model that has been adopted by a number of OSS projects both within and outside of Apache. I like that it provides visibility to PR authors and associates motivated experts to review and merge changes. Anthony > On Nov 19, 2020, at 10:46 AM, Ernie Burghardt wrote: > > Perfect, then let's give this a try. > +1 > > On 11/19/20, 10:45 AM, "Robert Houghton" wrote: > >Hi Ernie, > >DRAFT PRs do not get reviewers by default, but when the draft transitions > to ‘ready’, then the owners are requested to review. > > >From: Ernie Burghardt >Date: Thursday, November 19, 2020 at 9:56 AM >To: dev@geode.apache.org >Subject: Re: [DISCUSS] Adding CODEOWNERS to Apache Geode >Does GitHub allow us to limit this automated action to non-DRAFT PRs? > >On 11/18/20, 8:28 PM, "Owen Nichols" wrote: > >+1 This will greatly improve the experience for contributors. Instead > of an intimidating empty list of reviewers when you submit a PR (and no > ability to add reviewers, if you’re not a committer), it will be great to > already have at least two reviewers automagically assigned. > >I have a small concern that initially populating this file via a > flurry of PRs may result in a lot of merge conflicts with anyone else that > volunteers on the same or an adjacent line. Also, since you _must_ be a > committer to be a code owner, is a PR even necessary…would directly > committing changes to the feature/introduce-codeowners branch be acceptable? > If not, who needs to review and who can merge the PRs against the ‘introduce’ > branch? > >What happens if you are the only owner for an area, can you approve > your own PR? Even if the goal is two owners per area, does that mean PRs by > either owner cannot be merged if the only other owner is on vacation or > otherwise unavailable? > >Can we submit PRs against the ‘introduce’ branch now and they just > won’t be merged before Nov 26, or do we all just need to be patient until > this review period has concluded? > >From: Robert Houghton >Date: Wednesday, November 18, 2020 at 2:07 PM >To: dev@geode.apache.org >Subject: [DISCUSS] Adding CODEOWNERS to Apache Geode >Hello Devs. > >I would like to improve the quality of the pull-request reviews we see > for >critical parts of the Apache Geode project. In discussions with other >committers, a (not the) big hurdle to that is getting the right eyes to >look at a given PR. To that end, I propose the adoption of GitHub's >CODEOWNERS functionality for the Apache Geode code repository. > >A discussion-document of this issue has been written up >by @upthewaterspout. Thanks Dan! > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FIntroduce%2BCodeowners%2Bfile&data=04%7C01%7Cbakera%40vmware.com%7C9ee8d7fc46874fbb128208d88cbb655c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414083771253757%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TFj61EnKBPrO1AfpH6BrVybUjzcnS%2BkymFMo1JxfhxY%3D&reserved=0 > >I have tested the feature with fellow Geode committers @upthewaterspout >and @onichols-pivotal, and found it to meet our expectations. Please >review the document, and comment or reply to this thread, by 25 > November, >so we might start the task of nominating and applying for ownership. > >-Robert Houghton >
Re: [PROPOSAL] Change the default value of conserve-sockets to false
I think there are many good reasons to flip the default value for this property. I do question whether requiring a user to allocate new hardware to support the changed resource requirements is appropriate for a minor version bump. In most cases I think that would come as an unwelcome surprise during the upgrade. Anthony > On Nov 19, 2020, at 10:42 AM, Dan Smith wrote: > > Personally, this has caused enough grief in the past (both ways, actually!) > that I 'd say this is a major version change. > I agree with John. Either value of conserve-sockets can crash or hang your > system depending on your use case. > > If this was just a matter of slowing down or speeding up performance, I think > we could change it. But users that are impacted won't just see their system > slow down. It will crash or hang. Potentially only with production sized > workloads. > > With conserve-sockets=false every thread on the server creates its own > sockets to other servers. With N servers that's N sockets per thread. With > our default of a max of 800 threads for client connections and a 20 server > cluster you are looking at a worst case of 800 * 20 = 16K sending sockets per > server, with another 16K receiving sockets and 16K receiving threads. That's > before considering function execution threads, WAN receivers, and various > other executors we have on the server. Users with too many threads will hit > their file descriptor or thread limits. Or they will run out of memory for > thread stacks, socket buffers, etc. > > -Dan >
Broken: apache/geode-native#2817 (support/1.13 - 6e372e5)
Build Update for apache/geode-native - Build: #2817 Status: Broken Duration: 1 min and 40 secs Commit: 6e372e5 (support/1.13) Author: Owen Nichols Message: parameterize GEODE_VERSION so that Geode release scripts can maintain this going forward View the changeset: https://github.com/apache/geode-native/compare/40bf0626a9a3...6e372e5c57c4 View the full build log and details: https://travis-ci.com/github/apache/geode-native/builds/202949906?utm_medium=notification&utm_source=email -- You can unsubscribe from build emails from the apache/geode-native repository going to https://travis-ci.com/account/preferences/unsubscribe?repository=16807653&utm_medium=notification&utm_source=email. Or unsubscribe from *all* email updating your settings at https://travis-ci.com/account/preferences/unsubscribe?utm_medium=notification&utm_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
Fixed: apache/geode-native#2818 (support/1.12 - 3478d98)
Build Update for apache/geode-native - Build: #2818 Status: Fixed Duration: 1 hr, 18 mins, and 26 secs Commit: 3478d98 (support/1.12) Author: Owen Nichols Message: parameterize GEODE_VERSION so that Geode release scripts can maintain this going forward View the changeset: https://github.com/apache/geode-native/compare/96137e90e527...3478d98a4993 View the full build log and details: https://travis-ci.com/github/apache/geode-native/builds/202952964?utm_medium=notification&utm_source=email -- You can unsubscribe from build emails from the apache/geode-native repository going to https://travis-ci.com/account/preferences/unsubscribe?repository=16807653&utm_medium=notification&utm_source=email. Or unsubscribe from *all* email updating your settings at https://travis-ci.com/account/preferences/unsubscribe?utm_medium=notification&utm_source=email. Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.
