New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
blockSize - a new config paramter for max size of the block #272
Changes from 4 commits
0317c7f
4ecadfb
dbb5555
88b5533
3df49f1
7c9a327
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,21 +3,19 @@ | |
import org.apache.commons.logging.Log; | ||
import org.apache.commons.logging.LogFactory; | ||
|
||
import zingg.block.Block; | ||
|
||
public class Heuristics { | ||
|
||
public static final Log LOG = LogFactory.getLog(Heuristics.class); | ||
|
||
public static long getMaxBlockSize(long totalCount) { | ||
public static final long MIN_SIZE = 8L; | ||
public static long getMaxBlockSize(long totalCount, long blockSizeFromConfig) { | ||
long maxSize = 8; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldnt this be set to the min size var ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Used MIN_SIZE |
||
/*if (totalCount > 100 && totalCount < 500){ | ||
maxSize = totalCount / 5; | ||
} | ||
else {*/ | ||
maxSize = (long) (0.001 * totalCount); | ||
LOG.debug("**Block size found **" + maxSize); | ||
if (maxSize > 100) maxSize = 100; | ||
LOG.debug("**Block size found **"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please print max size here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restored |
||
if (maxSize > blockSizeFromConfig) maxSize = blockSizeFromConfig; | ||
if (maxSize <= 8) maxSize = 8; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use the defined constant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. used MIN_SIZE |
||
//} | ||
LOG.info("**Block size **" + maxSize + " and total count was " + totalCount); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package zingg.util; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
||
public class TestHeuristics { | ||
long blockSizeConfigured = 50L; | ||
|
||
@Test | ||
public void testMaxBlockSizeWhenCalculatedValueIsMoreThanConfigValue() throws Throwable { | ||
long size = Heuristics.getMaxBlockSize(70000, blockSizeConfigured); | ||
assertEquals(size, 50); | ||
} | ||
|
||
@Test | ||
public void testMaxBlockSizeWhenCalculatedValueIsLessThanConfigValueButMoreThanMinValue() throws Throwable { | ||
long size = Heuristics.getMaxBlockSize(25000, blockSizeConfigured); | ||
assertEquals(size, 25); | ||
} | ||
|
||
@Test | ||
public void testMaxBlockSizeWhenCalculatedValueIsLessThanMinValue() throws Throwable { | ||
long size = Heuristics.getMaxBlockSize(5000, blockSizeConfigured); | ||
assertEquals(size, Heuristics.MIN_SIZE); | ||
} | ||
|
||
@Test | ||
public void testMaxBlockSizeWhenConfigValueItselfIsLessThanMinValue() throws Throwable { | ||
long blockSizeConfigured = 5L; | ||
long size = Heuristics.getMaxBlockSize(25000, blockSizeConfigured); | ||
assertEquals(size, Heuristics.MIN_SIZE); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
camel casing is missing - getBlockSize, same for setter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will break the json parsing - please also test one end to end case with 120l records and set block size in args. Use debug logs to verify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to Camel Case.
Ran with examples/febrl120k/config.json. Output attached. Appropriate block size was selected.