Skip to content

Conversation

@ericl
Copy link
Contributor

@ericl ericl commented Jun 29, 2016

What changes were proposed in this pull request?

This patches MemoryAllocator to fill clean and freed memory with known byte values, similar to https://github.com/jemalloc/jemalloc/wiki/Use-Case:-Find-a-memory-corruption-bug . Memory filling is flag-enabled in test only by default.

How was this patch tested?

Unit test that it's on in test.

cc @sameeragarwal

@SparkQA
Copy link

SparkQA commented Jun 30, 2016

Test build #61510 has finished for PR 13983 at commit ad707ac.

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

@sameeragarwal
Copy link
Member

LGTM

@rxin
Copy link
Contributor

rxin commented Jul 6, 2016

Merging in master.

@asfgit asfgit closed this in 44c7c62 Jul 6, 2016

@Test
public void memoryDebugFillEnabledInTest() {
Assert.assertTrue(MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED);
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion fails in Jenkins.
Did you intend to set the flag to true in a static block at the beginning of the test ?

Copy link
Contributor

@rxin rxin Jul 7, 2016

Choose a reason for hiding this comment

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

Yea I think the problem is that Maven doesn't have the property set, because it is set only in SBT.

cc @ericl we need to set that in Maven too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it's fixed by #14084

ghost pushed a commit to dbtsai/spark that referenced this pull request Jul 7, 2016
## What changes were proposed in this pull request?

Fixed the maven build for apache#13983

## How was this patch tested?

The existing tests.

Author: Shixiong Zhu <shixiong@databricks.com>

Closes apache#14084 from zsxwing/fix-maven.
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