kfaraz commented on code in PR #13581:
URL: https://github.com/apache/druid/pull/13581#discussion_r1050415916
##########
examples/bin/start-druid-main.py:
##########
@@ -436,43 +428,39 @@ def check_memory_constraint(total_memory, services):
return int(total_memory * 0.8)
-def build_mm_task_java_opts_array(memory_type):
- task_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
-
- mem_array = TASK_MEM_MAP.get(memory_type)
+def build_mm_task_java_opts_array(task_memory):
+ tasks_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
+ mem_array = ["-Xms{0}m".format(task_memory),
"-Xmx{0}m".format(task_memory),
"-XX:MaxDirectMemorySize={0}m".format(task_memory)]
java_opts_list = TASK_JAVA_OPTS_ARRAY + mem_array
-
for item in java_opts_list:
- task_memory += '\"{0}\",'.format(item)
+ tasks_memory += '\"{0}\",'.format(item)
- task_memory = task_memory[:-1]
- task_memory += ']'
- return task_memory
+ tasks_memory = tasks_memory[:-1]
+ tasks_memory += ']'
Review Comment:
style nit: it might be easier to read if this line is replaced with:
```suggestion
tasks_memory += '-D{0}=[{1}]'.format(TASK_JAVA_OPTS_PROPERTY,
task_java_opts_value)
```
and line 432 can be removed.
##########
examples/bin/start-druid-main.py:
##########
@@ -436,43 +428,39 @@ def check_memory_constraint(total_memory, services):
return int(total_memory * 0.8)
-def build_mm_task_java_opts_array(memory_type):
- task_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
-
- mem_array = TASK_MEM_MAP.get(memory_type)
+def build_mm_task_java_opts_array(task_memory):
+ tasks_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
Review Comment:
The similar names `task_memory`, `tasks_memory` seem a little confusing.
Maybe rename this field to `task_java_opts_string` or `task_java_opts_value`.
##########
examples/bin/start-druid-main.py:
##########
@@ -436,43 +428,39 @@ def check_memory_constraint(total_memory, services):
return int(total_memory * 0.8)
-def build_mm_task_java_opts_array(memory_type):
- task_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
-
- mem_array = TASK_MEM_MAP.get(memory_type)
+def build_mm_task_java_opts_array(task_memory):
+ tasks_memory = '-D{0}=['.format(TASK_JAVA_OPTS_PROPERTY)
+ mem_array = ["-Xms{0}m".format(task_memory),
"-Xmx{0}m".format(task_memory),
"-XX:MaxDirectMemorySize={0}m".format(task_memory)]
java_opts_list = TASK_JAVA_OPTS_ARRAY + mem_array
-
for item in java_opts_list:
- task_memory += '\"{0}\",'.format(item)
+ tasks_memory += '\"{0}\",'.format(item)
- task_memory = task_memory[:-1]
- task_memory += ']'
- return task_memory
+ tasks_memory = tasks_memory[:-1]
+ tasks_memory += ']'
+ return tasks_memory
def compute_tasks_memory(allocated_memory):
- if allocated_memory >= 4096:
- task_count = int(allocated_memory / 2048)
- memory_type = TASK_MEM_TYPE_HIGH
- task_memory_mb = 2048
+ cpu_count = multiprocessing.cpu_count()
+
+ if allocated_memory >= cpu_count * 1024:
+ task_count = cpu_count
+ task_memory_mb = min(2048, int(allocated_memory / cpu_count))
elif allocated_memory >= 2048:
task_count = int(allocated_memory / 1024)
- memory_type = TASK_MEM_TYPE_MEDIUM
task_memory_mb = 1024
Review Comment:
The `task_memory_mb` returned here is now being used as is to format the Xmx
and direct. So we should be returning half the value here now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
