Skip to content

PARQUET-373: Fix flaky MemoryManager tests. - #269

Closed
rdblue wants to merge 1 commit into
apache:masterfrom
rdblue:PARQUET-373-fix-flaky-mem-manager-tests
Closed

PARQUET-373: Fix flaky MemoryManager tests.#269
rdblue wants to merge 1 commit into
apache:masterfrom
rdblue:PARQUET-373-fix-flaky-mem-manager-tests

Conversation

@rdblue

@rdblue rdblue commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

No description provided.

@rdblue

rdblue commented Sep 24, 2015

Copy link
Copy Markdown
Contributor Author

@spena could you take a look at this? The MemoryManager is in support of Hive.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are we deleting this file after it's being used? Mabye a file.deleteOnExit() here would help to avoid leaving temporary files after tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing this file is done by the TemporaryFolder. Because we use the Rule annotation, it is created and cleaned up by the framework.

@rdblue
rdblue force-pushed the PARQUET-373-fix-flaky-mem-manager-tests branch from 5e8afc3 to b88dd3d Compare September 25, 2015 21:17
@spena

spena commented Sep 25, 2015

Copy link
Copy Markdown

Thanks for the explanation. The code looks good to me.
+1

@rdblue

rdblue commented Oct 5, 2015

Copy link
Copy Markdown
Contributor Author

@isnotinvain could you take a look? Lots of PR tests are failing because of this so I'd like to get it in.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick, but:

if (!file.delete()) { throw new RuntimeException("Could not delete file: " + file); }

@isnotinvain

Copy link
Copy Markdown
Contributor

+1, one minor comment

@rdblue
rdblue force-pushed the PARQUET-373-fix-flaky-mem-manager-tests branch from b88dd3d to 1b55889 Compare October 19, 2015 22:21
@asfgit asfgit closed this in 5294c64 Oct 19, 2015
@rdblue

rdblue commented Oct 19, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing this, @isnotinvain!

rdblue added a commit to rdblue/parquet-mr that referenced this pull request Jul 13, 2016
Author: Ryan Blue <blue@apache.org>

Closes apache#269 from rdblue/PARQUET-373-fix-flaky-mem-manager-tests and squashes the following commits:

1b55889 [Ryan Blue] PARQUET-373: Fix flaky MemoryManager tests.
rdblue added a commit to rdblue/parquet-mr that referenced this pull request Jan 6, 2017
Author: Ryan Blue <blue@apache.org>

Closes apache#269 from rdblue/PARQUET-373-fix-flaky-mem-manager-tests and squashes the following commits:

1b55889 [Ryan Blue] PARQUET-373: Fix flaky MemoryManager tests.
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.

3 participants