You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2023/10/04 10:59:10 UTC
[kudu-CR] WIP [CLI] Set rpc max message size per available memory to accomodate huge response payloads
Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20535
Change subject: WIP [CLI] Set rpc_max_message_size per available memory to accomodate huge response payloads
......................................................................
WIP [CLI] Set rpc_max_message_size per available memory to accomodate huge response payloads
It has been observed that for certain Kudu CLI commands response payload
size is too big to fit in the current default rpc message size limit of 50MiB.
This patch add logic to set the value of rpc message max size for Kudu CLI based
on maximum available memory or maximum possible rpc message size limit of 2GiB.
This would help accomodate that extra heavy response payload.
Along with increasing default value of rpc_max_message_size, default value
of tablet_transaction_memory_limit_mb is also set to atleast same value as
rpc max message size to pass the group validation check.
This change of default flag values is only applicable to Kudu CLI tool.
Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
---
M src/kudu/tools/tool_main.cc
1 file changed, 32 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/20535/1
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20535
to look at the new patch set (#4).
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
[CLI] Set rpc_max_message_size to accommodate huge response payloads
It has been observed that for certain Kudu CLI commands response
payload size is too big to fit in the current default RPC message size
limit of 50MiB. This patch adds logic to set the value of RPC message
max size for Kudu CLI based on maximum available memory or maximum
possible RPC message size limit of 2GiB. This would help accommodate
that extra heavy response payload.
Along with increasing default value of rpc_max_message_size, default
value of tablet_transaction_memory_limit_mb is also set to at least same
value as rpc_max_message_size to pass the group validation check.
This change of default flag values is only applicable to Kudu CLI tool.
Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
---
M src/kudu/tools/tool_main.cc
M src/kudu/util/process_memory.cc
M src/kudu/util/process_memory.h
3 files changed, 51 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/20535/4
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP [CLI] Set rpc max message size per available memory to accomodate huge response payloads
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: WIP [CLI] Set rpc_max_message_size per available memory to accomodate huge response payloads
......................................................................
Patch Set 1: Verified+1
(20 comments)
Thank you for the patch!
Overall looks good to me, just a few nits.
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG
Commit Message:
PS1:
For git commit messages, please follow the 'generic git commit guidelines and good practices' that are referenced from this Kudu doc page: https://kudu.apache.org/docs/contributing.html#_submitting_patches
In particular, it makes sense to keep the line length in the commit description under 72 symbols (BTW, gerrit show the ruler at the right marging that's specific for commit messages). Another good practice is keeping the subject/summary under 50 symbols, if possible (it's OK to sacrifice grammatical correctness for that -- similar to what one would see in newspapers' headlines).
You could find the rationale behind in the 'generic git commit guidelines and good practices', and I'm adding a link here just in case:
https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines
Thank you!
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@7
PS1, Line 7: accomodate
nit: accommodate
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@10
PS1, Line 10: rpc
nit: RPC
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@11
PS1, Line 11: add
nit: adds
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@11
PS1, Line 11: rpc
nit: RPC
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@12
PS1, Line 12: rpc
nit: RPC
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@13
PS1, Line 13: accomodate
nit: accommodate
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@16
PS1, Line 16: atleast
nit: at least
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@255
PS1, Line 255: or
nit: and
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@255
PS1, Line 255: d
nit: add a period in the end of the sentence
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@256
PS1, Line 256: s
nit: add a period in the end of the sentence
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@256
PS1, Line 256: Return value
nit: The return value ?
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@260
PS1, Line 260: kudu::process_memory::CurrentConsumption())
For simplicity, consider dropping this part: I guess it's not going to be different from 0 if called as of now in the main() function of the CLI tools.
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@274
PS1, Line 274: rpc
nit: RPC
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@275
PS1, Line 275: a rpc
nit: an RPC ?
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@275
PS1, Line 275: accomodate
accommodate
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@277
PS1, Line 277: int64_t
nit: consider adding 'const' to express that this is an invariant in this scope
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@283
PS1, Line 283: atleast
nit: at least ?
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@284
PS1, Line 284: rpc
nit: RPC
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@286
PS1, Line 286: int64_t
nit: consider adding 'const' to express that this is an invariant in this scope
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 04 Oct 2023 18:30:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20535
to look at the new patch set (#3).
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
[CLI] Set rpc_max_message_size to accommodate huge response payloads
It has been observed that for certain Kudu CLI commands response
payload size is too big to fit in the current default RPC message size
limit of 50MiB. This patch adds logic to set the value of RPC message
max size for Kudu CLI based on maximum available memory or maximum
possible RPC message size limit of 2GiB. This would help accommodate
that extra heavy response payload.
Along with increasing default value of rpc_max_message_size, default
value of tablet_transaction_memory_limit_mb is also set to at least same
value as rpc_max_message_size to pass the group validation check.
This change of default flag values is only applicable to Kudu CLI tool.
Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
---
M src/kudu/tools/tool_main.cc
M src/kudu/util/process_memory.cc
M src/kudu/util/process_memory.h
3 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/20535/3
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
[CLI] Set rpc_max_message_size to accommodate huge response payloads
It has been observed that for certain Kudu CLI commands response
payload size is too big to fit in the current default RPC message size
limit of 50MiB. This patch adds logic to set the value of RPC message
max size for Kudu CLI based on maximum available memory or maximum
possible RPC message size limit of 2GiB. This would help accommodate
that extra heavy response payload.
Along with increasing default value of rpc_max_message_size, default
value of tablet_transaction_memory_limit_mb is also set to at least same
value as rpc_max_message_size to pass the group validation check.
This change of default flag values is only applicable to Kudu CLI tool.
Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Reviewed-on: http://gerrit.cloudera.org:8080/20535
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/tools/tool_main.cc
M src/kudu/util/process_memory.cc
M src/kudu/util/process_memory.h
3 files changed, 51 insertions(+), 4 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 5
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc@264
PS2, Line 264: int64_t max_memory_available = kudu::process_memory::MaxMemoryAvailable();
> Note: I noticed from a couple of test failures (RaftConsensusITest.TestEarl
I guess the observed behavior might be related to the fact that current test mini-cluster uses the kudu CLI to start kudu-master and kudu-tserver for tests: `kudu test mini_cluster ...`.
http://gerrit.cloudera.org:8080/#/c/20535/3/src/kudu/util/process_memory.cc
File src/kudu/util/process_memory.cc:
http://gerrit.cloudera.org:8080/#/c/20535/3/src/kudu/util/process_memory.cc@219
PS3, Line 219: int64_t MaxMemoryAvailable() {
: int64_t total_ram;
: CHECK_OK(Env::Default()->GetTotalRAMBytes(&total_ram));
: // We will use 80% of system RAM to align with default hard limit.
: total_ram = total_ram * 4;
: total_ram /= 5;
:
: return total_ram;
: }
Once this new function is introduced, does it make sense to use it in DoInitLimits() instead of duplicating the same code here and there?
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 09 Oct 2023 22:37:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 10 Oct 2023 19:02:02 +0000
Gerrit-HasComments: No
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20535
to look at the new patch set (#2).
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
[CLI] Set rpc_max_message_size to accommodate huge response payloads
It has been observed that for certain Kudu CLI commands response
payload size is too big to fit in the current default RPC message size
limit of 50MiB. This patch adds logic to set the value of RPC message
max size for Kudu CLI based on maximum available memory or maximum
possible RPC message size limit of 2GiB. This would help accommodate
that extra heavy response payload.
Along with increasing default value of rpc_max_message_size, default
value of tablet_transaction_memory_limit_mb is also set to at least same
value as rpc_max_message_size to pass the group validation check.
This change of default flag values is only applicable to Kudu CLI tool.
Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
---
M src/kudu/tools/tool_main.cc
M src/kudu/util/process_memory.cc
M src/kudu/util/process_memory.h
3 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/20535/2
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc@264
PS2, Line 264: int64_t max_memory_available = kudu::process_memory::MaxMemoryAvailable();
Note: I noticed from a couple of test failures (RaftConsensusITest.TestEarlyCommitDespiteMemoryPressure and ClientStressLowMemoryTest.TestMemoryThrottling) that if I use kudu::process_memory::HardLimit here, it fiddles with memory_limit_hard_bytes that is set to low values by above two tests to induce memory pressure. When HardLimit is called from here (to get the memory available), it overwrites the memory_limit_hard_bytes flag to a bigger value (i.e. the actual memory available) thereby causing the test to fail because memory pressure condition was not induced to arrive at the expected result.
Due to the time constraint, I was not able to RCA why HardLimit would overwrite the hard limit flag. HardLimit implementation does initialise g_hard_limit but it is not clear how that would impact the server's flag. IIUC, Kudu tool and server are separate binaries, so even if I hard limit gets overwritten upon calling HardLimit in Kudu CLI code, it should only impact its own copy of hard limit flag and not the one within the server. Maybe I am missing something here.
If this is not expected behaviour, I would like to open a separate jira to address this.
For now, to make it work, I have added a new method whose only job is to find out the memory available without initialising any limits.
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Oct 2023 10:39:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP [CLI] Set rpc max message size per available memory to accomodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: WIP [CLI] Set rpc_max_message_size per available memory to accomodate huge response payloads
......................................................................
Patch Set 1:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG
Commit Message:
PS1:
> For git commit messages, please follow the 'generic git commit guidelines a
Thank you, Alexey.
I am anyway going to re-arrange (when I will make all these corrections) this a little bit before removing WIP.
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@7
PS1, Line 7: accomodate
> nit: accommodate
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@10
PS1, Line 10: rpc
> nit: RPC
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@11
PS1, Line 11: add
> nit: adds
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@11
PS1, Line 11: rpc
> nit: RPC
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@12
PS1, Line 12: rpc
> nit: RPC
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@13
PS1, Line 13: accomodate
> nit: accommodate
Done
http://gerrit.cloudera.org:8080/#/c/20535/1//COMMIT_MSG@16
PS1, Line 16: atleast
> nit: at least
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@255
PS1, Line 255: or
> nit: and
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@255
PS1, Line 255: d
> nit: add a period in the end of the sentence
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@256
PS1, Line 256: Return value
> nit: The return value ?
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@256
PS1, Line 256: s
> nit: add a period in the end of the sentence
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@260
PS1, Line 260: kudu::process_memory::CurrentConsumption())
> For simplicity, consider dropping this part: I guess it's not going to be d
In my tests, the value comes around 600K - 700K usually. But that is still negligible as compared to HardLimit which is in GBs and that is usually going to be the case. So, it should be ok to ignore CurrentConsumption.
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@274
PS1, Line 274: rpc
> nit: RPC
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@275
PS1, Line 275: a rpc
> nit: an RPC ?
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@275
PS1, Line 275: accomodate
> accommodate
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@277
PS1, Line 277: int64_t
> nit: consider adding 'const' to express that this is an invariant in this s
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@283
PS1, Line 283: atleast
> nit: at least ?
Done
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@284
PS1, Line 284: rpc
> nit: RPC
used exact flag name instead i.e. rpc_max_message_size
http://gerrit.cloudera.org:8080/#/c/20535/1/src/kudu/tools/tool_main.cc@286
PS1, Line 286: int64_t
> nit: consider adding 'const' to express that this is an invariant in this s
Done
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Oct 2023 09:11:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [CLI] Set rpc max message size to accommodate huge response payloads
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20535 )
Change subject: [CLI] Set rpc_max_message_size to accommodate huge response payloads
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
http://gerrit.cloudera.org:8080/#/c/20535/2/src/kudu/tools/tool_main.cc@264
PS2, Line 264: int64_t max_memory_available = kudu::process_memory::MaxMemoryAvailable();
> I guess the observed behavior might be related to the fact that current tes
That's a possibility. Would need to verify that.
http://gerrit.cloudera.org:8080/#/c/20535/3/src/kudu/util/process_memory.cc
File src/kudu/util/process_memory.cc:
http://gerrit.cloudera.org:8080/#/c/20535/3/src/kudu/util/process_memory.cc@219
PS3, Line 219: int64_t MaxMemoryAvailable() {
: int64_t total_ram;
: CHECK_OK(Env::Default()->GetTotalRAMBytes(&total_ram));
: // We will use 80% of system RAM to align with default hard limit.
: total_ram = total_ram * 4;
: total_ram /= 5;
:
: return total_ram;
: }
> Once this new function is introduced, does it make sense to use it in DoIni
Yeah, that makes sense.
Thanks for pointing that out.
--
To view, visit http://gerrit.cloudera.org:8080/20535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27b494bc1fde46c2a095c7291fc840a98429068
Gerrit-Change-Number: 20535
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 10 Oct 2023 15:21:51 +0000
Gerrit-HasComments: Yes