You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2023/02/26 09:38:49 UTC
[GitHub] [incubator-uniffle] advancedxy opened a new pull request, #662: [#642]feat(server): better default options for shuffle server
advancedxy opened a new pull request, #662:
URL: https://github.com/apache/incubator-uniffle/pull/662
### What changes were proposed in this pull request?
1. `rss.server.buffer.capacity` uses JVM heap size * ratio(0.8) by default
2. `rss.server.read.buffer.capacity` uses JVM heap size * ratio(0.4) by default
3. `rss.server.disk.capacity` uses disk space * ratio(0.9) by default
### Why are the changes needed?
Fix: #642
### Does this PR introduce _any_ user-facing change?
Yes. Three new configurations are introduced, users can specify ratio values
for buffer, read buffer and disk capacity
### How was this patch tested?
New UTs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1446221682
> > @zuston do you have any suggestion ratio for buffer and read buffer then.
>
> For us, read buffer size is the half of buffer capacity size. But from the dashboard, the read buffer size could be set to smaller.
> > @zuston do you have any suggestion ratio for buffer and read buffer then.
>
> For us, read buffer size is the half of buffer capacity size. But from the dashboard, the read buffer size could be set to smaller.
I just changed the default value of buffer capacity and read capacity ratio to 0.6 and 0.2, WDYT? @zuston @jerqi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445644080
Our memory is equal to `rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445895564
> @zuston do you have any suggestion ratio for buffer and read buffer then.
For us, read buffer size is the half of buffer capacity size. But from the dashboard, the read buffer size could be set to smaller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445759237
> > Our memory is equal to `rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program`.
>
> If I understand the shuffle server code correctly, read buffer and buffer capacity have some overlapping region, So it's possible `rss.server.buffer.capacity` + `rss.server.read.buffer.capacity` > 1.0.
>
> The current default value may a bit larger consider the meta memory required for shuffle server.
They shouldn't have overlap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445835441
> > > > Our memory is equal to `rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program`.
> > >
> > >
> > > If I understand the shuffle server code correctly, read buffer and buffer capacity have some overlapping region, So it's possible `rss.server.buffer.capacity` + `rss.server.read.buffer.capacity` > 1.0.
> > > The current default value may a bit larger consider the meta memory required for shuffle server.
> >
> >
> > They shouldn't have overlap.
>
> IIUC, the buffer caches data in memory, when reading with memory shuffle, the data size is calculated both for buffer and read buffer, these two regions should be considered overlapped region?
No, they will copy memory. They shouldn't overlap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1446344878
Thanks, @zuston and @jerqi. I'm merging this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445829275
> Our memory is equal to rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program.
From the point of view of production environment, the reserved system memory should be the half of xmx. If not, the full gc will occur when the server meets the high pressure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy merged pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy merged PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445311433
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#662](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e19cb82) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/c70d1543e7c9ca774e986026890763ce949a10e5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c70d154) will **increase** coverage by `1.21%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #662 +/- ##
============================================
+ Coverage 60.67% 61.88% +1.21%
+ Complexity 1802 1712 -90
============================================
Files 216 193 -23
Lines 12458 9955 -2503
Branches 1052 1012 -40
============================================
- Hits 7559 6161 -1398
+ Misses 4494 3461 -1033
+ Partials 405 333 -72
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/uniffle/server/ShuffleServerConf.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyQ29uZi5qYXZh) | `99.37% <100.00%> (+0.03%)` | :arrow_up: |
| [...he/uniffle/server/buffer/ShuffleBufferManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9idWZmZXIvU2h1ZmZsZUJ1ZmZlck1hbmFnZXIuamF2YQ==) | `83.50% <100.00%> (+0.75%)` | :arrow_up: |
| [...he/uniffle/server/storage/LocalStorageManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9zdG9yYWdlL0xvY2FsU3RvcmFnZU1hbmFnZXIuamF2YQ==) | `86.59% <100.00%> (+0.15%)` | :arrow_up: |
| [...rg/apache/uniffle/storage/common/LocalStorage.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmFnZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdW5pZmZsZS9zdG9yYWdlL2NvbW1vbi9Mb2NhbFN0b3JhZ2UuamF2YQ==) | `55.41% <100.00%> (+4.09%)` | :arrow_up: |
| [.../java/org/apache/uniffle/server/ShuffleServer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyLmphdmE=) | `62.06% <0.00%> (-1.98%)` | :arrow_down: |
| [.../apache/uniffle/coordinator/ClientConfManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29vcmRpbmF0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3VuaWZmbGUvY29vcmRpbmF0b3IvQ2xpZW50Q29uZk1hbmFnZXIuamF2YQ==) | `91.54% <0.00%> (-1.41%)` | :arrow_down: |
| [...org/apache/spark/shuffle/writer/AddBlockEvent.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvQWRkQmxvY2tFdmVudC5qYXZh) | | |
| [deploy/kubernetes/operator/pkg/utils/config.go](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL2NvbmZpZy5nbw==) | | |
| [deploy/kubernetes/operator/pkg/utils/util.go](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVwbG95L2t1YmVybmV0ZXMvb3BlcmF0b3IvcGtnL3V0aWxzL3V0aWwuZ28=) | | |
| [...org/apache/spark/shuffle/RssSparkShuffleUtils.java](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS9Sc3NTcGFya1NodWZmbGVVdGlscy5qYXZh) | | |
| ... and [21 more](https://codecov.io/gh/apache/incubator-uniffle/pull/662?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445885924
> > Our memory is equal to rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program.
>
> From the point of view of production environment, the reserved system memory should be the half of xmx. If not, the full gc will occur when the server meets the high pressure.
@zuston do you have any suggestion ratio for buffer and read buffer then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445885115
> No, they will copy memory. They shouldn't overlap.
Oh, I saw the code... This seems inefficient, let's reconsider this when doing off-heap memory management.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445755348
> Our memory is equal to `rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program`.
If I understand the shuffle server code correctly, read buffer and buffer capacity have some overlapping region, So it's possible `rss.server.buffer.capacity` + `rss.server.read.buffer.capacity` > 1.0.
The current default value may a bit larger consider the meta memory required for shuffle server.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445819086
> > > Our memory is equal to `rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program`.
> >
> >
> > If I understand the shuffle server code correctly, read buffer and buffer capacity have some overlapping region, So it's possible `rss.server.buffer.capacity` + `rss.server.read.buffer.capacity` > 1.0.
> > The current default value may a bit larger consider the meta memory required for shuffle server.
>
> They shouldn't have overlap.
IIUC, the buffer caches data in memory, when reading with memory shuffle, the data size is calculated both for buffer and read buffer, these two regions should be considered overlapped region?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #662: [#642]feat(server): better default options for shuffle server
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #662:
URL: https://github.com/apache/incubator-uniffle/pull/662#issuecomment-1445834145
> > Our memory is equal to rss.server.buffer.capacity + rss.server.read.buffer.capacity + extra memory used for program.
>
> From the point of view of production environment, the reserved system memory should be the half of xmx. If not, the full gc will occur when the server meets the high pressure.
Ouch, that hurts....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org