Skip to content

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented Apr 9, 2014

...g file.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@pwendell
Copy link
Contributor

/cc @andrewor14

@andrewor14
Copy link
Contributor

Good catch. The existing code fails when the logDir includes file://. This LGTM.

@andrewor14
Copy link
Contributor

Jenkins, test this please.

1 similar comment
@pwendell
Copy link
Contributor

Jenkins, test this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14009/

@vanzin
Copy link
Contributor Author

vanzin commented Apr 11, 2014

FYI: just found another issue in this same code. It does not handle the case where the default fs is not "file:", and will try to use FileOutputStream even when it's something like hdfs. I'll fix that and push the new code.

@andrewor14
Copy link
Contributor

Not sure if I understand. Are you saying that if we somehow configure the default file scheme to be hdfs://, then it will still match the null in the first case statement, even if the path is really for HDFS? Currently the FileLogger requires you to explicitly specify the hdfs:// scheme as it does not have a notion of default file schemes (unlike in say sc.textFile), though this need not be the case.

@vanzin
Copy link
Contributor Author

vanzin commented Apr 11, 2014

Ok, maybe it's not a bug per se. But I think it somewhat breaks the user's expectations.

Let's say that given a user's configuration, the default fs is hdfs; I'd expect that if I specify the log directory to be "/user/vanzin/logs", that would be in HDFS, not in the local fs. But with the current FileLogger code, it will assume that is a local path, since there is no scheme defined.

Anyway, the current patch fixes the actual bug; if you think the above should be handled too I have the code to do it working, but it's not strictly necessary.

@vanzin
Copy link
Contributor Author

vanzin commented Apr 11, 2014

Commit that implements what I'm suggesting, if you're interested:
vanzin@15c4d1e

I haven't sent a pull request for that one.

@andrewor14
Copy link
Contributor

@vanzin I recently experienced what you mean on a YARN cluster myself. I've looked at your patch and I think it's a good fix for Spark on YARN. Could you submit a PR for it? It will be more convenient for me to leave my comments there.

@andrewor14
Copy link
Contributor

On a separate note, @pwendell this PR is ready for merge.

@pwendell
Copy link
Contributor

Thanks, I've merged this.

@asfgit asfgit closed this in ac164b7 Apr 22, 2014
asfgit pushed a commit that referenced this pull request Apr 22, 2014
… lo...

...g file.

Author: Marcelo Vanzin <[email protected]>

Closes #375 from vanzin/event-file and squashes the following commits:

f673029 [Marcelo Vanzin] [SPARK-1459] Use local path (and not complete URL) when opening local log file.
(cherry picked from commit ac164b7)

Signed-off-by: Patrick Wendell <[email protected]>
vanzin pushed a commit to vanzin/spark that referenced this pull request Apr 22, 2014
This is related to SPARK-1459 / PR apache#375. Without this fix,
FileLogger.createLogDir() may try to create the log dir on
HDFS, while createWriter() will try to open the log file on
the local file system, leading to interesting errors and
confusion.
asfgit pushed a commit that referenced this pull request Apr 23, 2014
This is related to SPARK-1459 / PR #375. Without this fix,
FileLogger.createLogDir() may try to create the log dir on
HDFS, while createWriter() will try to open the log file on
the local file system, leading to interesting errors and
confusion.

Author: Marcelo Vanzin <[email protected]>

Closes #450 from vanzin/event-file-2 and squashes the following commits:

592cdb3 [Marcelo Vanzin] Honor default fs name when initializing event logger.
asfgit pushed a commit that referenced this pull request Apr 23, 2014
This is related to SPARK-1459 / PR #375. Without this fix,
FileLogger.createLogDir() may try to create the log dir on
HDFS, while createWriter() will try to open the log file on
the local file system, leading to interesting errors and
confusion.

Author: Marcelo Vanzin <[email protected]>

Closes #450 from vanzin/event-file-2 and squashes the following commits:

592cdb3 [Marcelo Vanzin] Honor default fs name when initializing event logger.
(cherry picked from commit dd1b7a6)

Signed-off-by: Patrick Wendell <[email protected]>
@vanzin vanzin deleted the event-file branch May 23, 2014 22:39
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
… lo...

...g file.

Author: Marcelo Vanzin <[email protected]>

Closes apache#375 from vanzin/event-file and squashes the following commits:

f673029 [Marcelo Vanzin] [SPARK-1459] Use local path (and not complete URL) when opening local log file.
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
This is related to SPARK-1459 / PR apache#375. Without this fix,
FileLogger.createLogDir() may try to create the log dir on
HDFS, while createWriter() will try to open the log file on
the local file system, leading to interesting errors and
confusion.

Author: Marcelo Vanzin <[email protected]>

Closes apache#450 from vanzin/event-file-2 and squashes the following commits:

592cdb3 [Marcelo Vanzin] Honor default fs name when initializing event logger.
tangzhankun pushed a commit to tangzhankun/spark that referenced this pull request Jul 25, 2017
erikerlandson pushed a commit to erikerlandson/spark that referenced this pull request Jul 28, 2017
mccheah pushed a commit to mccheah/spark that referenced this pull request Nov 28, 2018
…mbol is a token (apache#375)

In the case where the offending symbol is a CommonToken, this PR increases the accuracy of the start and stop origin by leveraging the start and stop index information from CommonToken.
mccheah pushed a commit to mccheah/spark that referenced this pull request Nov 28, 2018
…mbol is a token (apache#375)

In the case where the offending symbol is a CommonToken, this PR increases the accuracy of the start and stop origin by leveraging the start and stop index information from CommonToken.
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
…pache#375)

This change add the job of
terraform-provider-huaweicloud-acceptance-test-fusioncloud

Closes-Bug: theopenlab/openlab#130
arjunshroff pushed a commit to arjunshroff/spark that referenced this pull request Nov 24, 2020
turboFei pushed a commit to turboFei/spark that referenced this pull request Nov 6, 2025
…quet (apache#375)

[SPARK-47368][SQL]][3.5] Remove inferTimestampNTZ config check in ParquetRo…

### What changes were proposed in this pull request?

The configuration `spark.sql.parquet.inferTimestampNTZ.enabled` is not related the parquet row converter.  This PR is the remove the config check `spark.sql.parquet.inferTimestampNTZ.enabled` in the ParquetRowConverter

### Why are the changes needed?

Bug fix.  Otherwise reading TimestampNTZ columns may fail when `spark.sql.parquet.inferTimestampNTZ.enabled` is disabled.
### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

New UT

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#45492 from gengliangwang/PR_TOOL_PICK_PR_45480_BRANCH-3.5.

Authored-by: Gengliang Wang <[email protected]>

Signed-off-by: Gengliang Wang <[email protected]>
Co-authored-by: Gengliang Wang <[email protected]>
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.

4 participants