[SPARK-42379][SS] Use FileSystem.exists in FileSystemBasedCheckpointFileManager.exists

[HeartSaVioR]

What changes were proposed in this pull request?

This PR proposes to use FileSystem.exists in FileSystemBasedCheckpointFileManager.exists, which is consistent with other methods in FileSystemBasedCheckpointFileManager.

This PR also removes the test case QueryExecutionErrorsSuite.FAILED_RENAME_PATH: rename when destination path already exists because the test relies on incorrect custom file system instance with non-symmetric implementation between FileSystemBasedCheckpointFileManager.exists vs FileSystem.exists.
(See detailed explanation from #39936 (comment))

Why are the changes needed?

Other methods in FileSystemBasedCheckpointFileManager already uses FileSystem.exists for all cases checking existence of the path.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

[HeartSaVioR]

cc. @cloud-fan @viirya

[dongjoon-hyun]

Although it's a small change, could you file a JIRA for trace-ability, please, @HeartSaVioR ?

[HeartSaVioR]

Done.

[dongjoon-hyun]

Could you check the UT failure?

  test("FAILED_RENAME_PATH: rename when destination path already exists") {
    withTempPath { p =>
      withSQLConf(
        "spark.sql.streaming.checkpointFileManagerClass" ->
          classOf[FileSystemBasedCheckpointFileManager].getName,

[HeartSaVioR]

I'd argue that the test case itself is incorrect. It's heavily bound to the internal implementation - it relies on custom filesystem instance which returns true for all exists() calls, but it does not apply to the getFileStatus() hence it will end up with "file does not exist".

That said, test only passes if filesystem behaves incorrectly. If we change the custom filesystem instance to also provide dummy status for all getFileStatus() calls, the test case will fail.

I don't know how we can test the exception in E2E query. As the exception class denotes, it mainly occurs from concurrent writes for same offset/commit log file. We should have created offset log file concurrently, between "determining start microbatch" and "writing offset log after planning", which is super hard from outside of the query.

I'd suggest just remove the test. WDYT?

[HyukjinKwon]

I am fine w/ removing it.

[cloud-fan]

+1 to remove it.

[HeartSaVioR]

Test removed. Will merge once the build passes.

[LuciferYang]

+1, LGTM

[viirya]

Sounds reasonable to remove it.

[dongjoon-hyun]

+1 with the final commit.

[HeartSaVioR]

Thanks, merging to master.