Skip to content

Conversation

@mgaido91
Copy link
Contributor

What changes were proposed in this pull request?

When creating a session directory, Thrift should create the parent directory (i.e. /tmp/base_session_log_dir) if it is not present. It is common that many tools delete empty directories, so the directory may be deleted. This can cause the session log to be disabled.

This was fixed in HIVE-12262: this PR brings it in Spark too.

How was this patch tested?

manual tests

@SparkQA
Copy link

SparkQA commented Jan 16, 2018

Test build #86195 has finished for PR 20281 at commit 7fc1e00.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jerryshao
Copy link
Contributor

LGTM. So looks like the fix is exactly the same as Hive.

@jerryshao
Copy link
Contributor

@cloud-fan would you please take another look on this?

@cloud-fan
Copy link
Contributor

LGTM, cc @gatorsmile

@jerryshao
Copy link
Contributor

@gatorsmile , do you have further comment?

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

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

LGTM

@Override
public void setOperationLogSessionDir(File operationLogRootDir) {
if (!operationLogRootDir.exists()) {
LOG.warn("The operation log root directory is removed, recreating:" +
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra space at the end of line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry @jiangxb1987 I can't see any extra space at the end of this line...

Copy link
Contributor

Choose a reason for hiding this comment

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

we need an extra space: ... recreating: " +

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry, now I understand, thanks! Yes, in Hive this space is still missing, I see only now. Thanks.

@SparkQA
Copy link

SparkQA commented Jan 19, 2018

Test build #86385 has finished for PR 20281 at commit 8b4eb1c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

thanks, merging to master/2.3!

asfgit pushed a commit that referenced this pull request Jan 19, 2018
## What changes were proposed in this pull request?

When creating a session directory, Thrift should create the parent directory (i.e. /tmp/base_session_log_dir) if it is not present. It is common that many tools delete empty directories, so the directory may be deleted. This can cause the session log to be disabled.

This was fixed in HIVE-12262: this PR brings it in Spark too.

## How was this patch tested?

manual tests

Author: Marco Gaido <[email protected]>

Closes #20281 from mgaido91/SPARK-23089.

(cherry picked from commit e41400c)
Signed-off-by: Wenchen Fan <[email protected]>
@asfgit asfgit closed this in e41400c Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants