Skip to content
This repository has been archived by the owner on Nov 14, 2022. It is now read-only.

Fixing one pmem path on AppDirect mode may cause the pmem initialization path to be empty Path #10

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

JustDoCoder
Copy link
Contributor

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

@JustDoCoder
Copy link
Contributor Author

After many tests, it is ensured that one pmem path is initialized normally without affecting the previous functions

Copy link

@winningsix winningsix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me.

@@ -194,7 +194,12 @@ private[spark] class BlockManager(
if (numaNodeId == -1) {
numaNodeId = executorId.toInt
}
val path = pmemInitialPaths(numaNodeId % 2)
var path:String =""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly changed.

val path_postfix = File.separator + s"executor_${executorId}" + File.pathSeparator
var file  = if (1 == pmemInitialPaths.size) {
  new File(pmemInitialPaths(0) + path_postfix)
} else {
  new File(pmemInitialPaths(numaNodeId % 2) + path_postfix)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winningsix I agree with this change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winningsix Please review, thank you.

@winningsix
Copy link

LGTM

@winningsix winningsix merged commit 032cbae into oap-project:master Feb 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants