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
[CARBONDATA-1179] Improve the Size calculation of Objects being added and managed in LRU cache #1038
[CARBONDATA-1179] Improve the Size calculation of Objects being added and managed in LRU cache #1038
Conversation
Can one of the admins verify this patch? |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2499/ |
@@ -92,13 +94,14 @@ protected void checkAndLoadTableBlocks(AbstractIndex tableBlock, | |||
TableBlockInfo blockInfo = tableBlockUniqueIdentifier.getTableBlockInfo(); | |||
long requiredMetaSize = CarbonUtil.calculateMetaSize(blockInfo); |
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.
Better we can remove CarbonUtil.calculateMetaSize(blockInfo)
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.
any particular reason? we are using the value to determine whether we need to load the block into memory.
@@ -233,8 +235,7 @@ private SegmentTaskIndexWrapper loadAndGetTaskIdToSegmentsMap( | |||
taskIdToTableBlockInfoMap.entrySet().iterator(); | |||
long requiredSize = | |||
calculateRequiredSize(taskIdToTableBlockInfoMap, absoluteTableIdentifier); | |||
segmentTaskIndexWrapper | |||
.setMemorySize(requiredSize + segmentTaskIndexWrapper.getMemorySize()); | |||
|
|||
boolean isAddedToLruCache = |
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.
Putting the segment cache object prior to loading the segment b-tree could
allow dirty read in case of concurrent query.
Better to add the cache object after finishing the segment load.
db6c074
to
d0ed692
Compare
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/128/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2702/ |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1carbondata-pr-spark-1.6/org.apache.carbondata:carbondata-core: 1 |
28b0274
to
56e4d4b
Compare
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/160/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2736/ |
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/161/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2737/ |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
56e4d4b
to
6deb674
Compare
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/198/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2777/ |
Refer to this link for build results (access rights to CI server needed): |
* @return | ||
*/ | ||
public boolean tryPut(String columnIdentifier, long requiredSize) { | ||
if (LOGGER.isDebugEnabled()) { |
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.
It can better remove entry so that temp block memory also with in LRU limit.
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.
handled
@@ -45,6 +49,20 @@ | |||
private static final LogService LOGGER = | |||
LogServiceFactory.getLogService(ReverseDictionaryCache.class.getName()); | |||
|
|||
private static long sizeOfEmptyDictChunks = |
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.
these can be static final
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.
updated
6deb674
to
a5776ba
Compare
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/205/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2784/ |
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1carbondata-pr-spark-1.6/org.apache.carbondata:carbondata-core: 1 |
a5776ba
to
377dee9
Compare
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2786/ |
Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/207/ |
Refer to this link for build results (access rights to CI server needed): |
LGTM |
No description provided.