[GitHub] [hadoop] tomscut commented on pull request #4209: HDFS-16550. Improper cache-size for journal node may cause cluster crash
tomscut commented on PR #4209: URL: https://github.com/apache/hadoop/pull/4209#issuecomment-1329975976 Thanks @xkrogen for your thoughtful advice. It makes perfect sense to me. I updated the code. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on pull request #4209: HDFS-16550. Improper cache-size for journal node may cause cluster crash
tomscut commented on PR #4209: URL: https://github.com/apache/hadoop/pull/4209#issuecomment-1328405738 > I am -1 on the PR as-is. We have publicly exposed the current config `dfs.journalnode.edit-cache-size.bytes`; we can't just rename it and change the behavior now. I also think there is a lot of value in being able to configure the cache size exactly, rather than as a fraction, but I do recognize the value in using a ratio as a helpful default (one less knob to tune). I would propose: > > * _Add_ (not replace) a new config `dfs.journalnode.edit-cache-size.fraction` (or `.ratio`? but either way I think we should maintain the `edit-cache-size` prefix) > * If `edit-cache-size.bytes` is set, use that value. Otherwise, use the value of `edit-cache-size.fraction * Runtime#maxMemory()`, which has a default value set. > * I would suggest 0.5 rather than 0.3 for the default value of `fraction` but am open to discussion there. > > This still does change the default behavior slightly, since before you would get a 1GB cache and now you get `-Xmx * 0.5`, but there is an easy way to preserve the old behavior and if you've explicitly configured the cache size (which you probably did, if you're using the feature) then there is no change. Hi @xkrogen @ZanderXu , I updated the code according to the suggestion, please take a look. Thanks! -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on pull request #4209: HDFS-16550. Improper cache-size for journal node may cause cluster crash
tomscut commented on PR #4209: URL: https://github.com/apache/hadoop/pull/4209#issuecomment-1328405489 Hi @xkrogen @ZanderXu , I updated the code according to the suggestion, please take a look. Thanks! -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on pull request #4209: HDFS-16550. Improper cache-size for journal node may cause cluster crash
tomscut commented on PR #4209: URL: https://github.com/apache/hadoop/pull/4209#issuecomment-1322873539 > I am -1 on the PR as-is. We have publicly exposed the current config `dfs.journalnode.edit-cache-size.bytes`; we can't just rename it and change the behavior now. I also think there is a lot of value in being able to configure the cache size exactly, rather than as a fraction, but I do recognize the value in using a ratio as a helpful default (one less knob to tune). I would propose: > > * _Add_ (not replace) a new config `dfs.journalnode.edit-cache-size.fraction` (or `.ratio`? but either way I think we should maintain the `edit-cache-size` prefix) > * If `edit-cache-size.bytes` is set, use that value. Otherwise, use the value of `edit-cache-size.fraction * Runtime#maxMemory()`, which has a default value set. > * I would suggest 0.5 rather than 0.3 for the default value of `fraction` but am open to discussion there. > > This still does change the default behavior slightly, since before you would get a 1GB cache and now you get `-Xmx * 0.5`, but there is an easy way to preserve the old behavior and if you've explicitly configured the cache size (which you probably did, if you're using the feature) then there is no change. Thank you for your comments and detailed suggestions. It is a good idea to add a new config. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] tomscut commented on pull request #4209: HDFS-16550. Improper cache-size for journal node may cause cluster crash
tomscut commented on PR #4209: URL: https://github.com/apache/hadoop/pull/4209#issuecomment-1321325256 Hi @tasanuma @ayushtkn , could you also please take a look? Thanks. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
