No need to override executor overhead memory when using comet unified memory manager

What is the problem the feature request solves?

We do not need to override executor overhead memory when using comet unified memory manager.

datafusion-comet/spark/src/main/scala/org/apache/spark/Plugins.scala

Lines 53 to 75 in 3d9cbd5

    
               if (shouldOverrideMemoryConf(sc.getConf)) { 
        
                 val execMemOverhead = if (sc.getConf.contains(EXECUTOR_MEMORY_OVERHEAD.key)) { 
        
                   sc.getConf.getSizeAsMb(EXECUTOR_MEMORY_OVERHEAD.key) 
        
                 } else { 
        
                   // By default, executorMemory * spark.executor.memoryOverheadFactor, with minimum of 384MB 
        
                   val executorMemory = sc.getConf.getSizeAsMb(EXECUTOR_MEMORY.key, EXECUTOR_MEMORY_DEFAULT) 
        
                   val memoryOverheadFactor = getMemoryOverheadFactor(sc.getConf) 
        
                   val memoryOverheadMinMib = getMemoryOverheadMinMib(sc.getConf) 
        
                   Math.max((executorMemory * memoryOverheadFactor).toLong, memoryOverheadMinMib) 
        
                 } 
        
                 val cometMemOverhead = CometSparkSessionExtensions.getCometMemoryOverheadInMiB(sc.getConf) 
        
                 sc.conf.set(EXECUTOR_MEMORY_OVERHEAD.key, s"${execMemOverhead + cometMemOverhead}M") 
        
                 val newExecMemOverhead = sc.getConf.getSizeAsMb(EXECUTOR_MEMORY_OVERHEAD.key) 
        
                 logInfo(s""" 
        
                    Overriding Spark memory configuration for Comet: 
        
                      - Spark executor memory overhead: ${execMemOverhead}MB 
        
                      - Comet memory overhead: ${cometMemOverhead}MB 
        
                      - Updated Spark executor memory overhead: ${newExecMemOverhead}MB 
        
                    """) 
        
               }

Describe the potential solution

Override executor overhead memory only when comet unified memory manager is not enabled

Additional context

No response

Navigation Menu

Repository navigation

No need to override executor overhead memory when using comet unified memory manager #1378

What is the problem the feature request solves?

Describe the potential solution

Additional context

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (shouldOverrideMemoryConf(sc.getConf)) {
	val execMemOverhead = if (sc.getConf.contains(EXECUTOR_MEMORY_OVERHEAD.key)) {
	sc.getConf.getSizeAsMb(EXECUTOR_MEMORY_OVERHEAD.key)
	} else {
	// By default, executorMemory * spark.executor.memoryOverheadFactor, with minimum of 384MB
	val executorMemory = sc.getConf.getSizeAsMb(EXECUTOR_MEMORY.key, EXECUTOR_MEMORY_DEFAULT)
	val memoryOverheadFactor = getMemoryOverheadFactor(sc.getConf)
	val memoryOverheadMinMib = getMemoryOverheadMinMib(sc.getConf)

	Math.max((executorMemory * memoryOverheadFactor).toLong, memoryOverheadMinMib)
	}

	val cometMemOverhead = CometSparkSessionExtensions.getCometMemoryOverheadInMiB(sc.getConf)
	sc.conf.set(EXECUTOR_MEMORY_OVERHEAD.key, s"${execMemOverhead + cometMemOverhead}M")
	val newExecMemOverhead = sc.getConf.getSizeAsMb(EXECUTOR_MEMORY_OVERHEAD.key)

	logInfo(s"""
	Overriding Spark memory configuration for Comet:
	- Spark executor memory overhead: ${execMemOverhead}MB
	- Comet memory overhead: ${cometMemOverhead}MB
	- Updated Spark executor memory overhead: ${newExecMemOverhead}MB
	""")
	}

No need to override executor overhead memory when using comet unified memory manager #1378

Description

What is the problem the feature request solves?

Describe the potential solution

Additional context

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions