[jira] [Commented] (IGNITE-13052) Calculate result of reserveHistoryForExchange in advance
[ https://issues.apache.org/jira/browse/IGNITE-13052?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130799#comment-17130799 ] Ignite TC Bot commented on IGNITE-13052: {panel:title=Branch: [pull/7911/head] Base: [master] : No blockers found!|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}{panel} [TeamCity *--> Run :: All* Results|https://ci.ignite.apache.org/viewLog.html?buildId=5373489&buildTypeId=IgniteTests24Java8_RunAll] > Calculate result of reserveHistoryForExchange in advance > > > Key: IGNITE-13052 > URL: https://issues.apache.org/jira/browse/IGNITE-13052 > Project: Ignite > Issue Type: Improvement >Reporter: Ivan Rakov >Assignee: Vladislav Pyatkov >Priority: Major > Original Estimate: 80h > Time Spent: 10m > Remaining Estimate: 79h 50m > > Method reserveHistoryForExchange() is called on every partition map exchange. > It's an expensive call: it requires iteration over the whole checkpoint > history with possible retrieve of GroupState from WAL (it's stored on heap > with SoftReference). On some deployments this operation can take several > minutes. > The idea of optimization is to calculate its result only on first PME > (ideally, even before first PME, on recovery stage), keep resulting map > (grpId, partId -> earlisetCheckpoint) on heap and update it if necessary. > From the first glance, the map should be updated: > 1) On checkpoint. If a new partition appears on local node, it should be > registered in the map with current checkpoint. If a partition is evicted from > local node, or changes its state to non-OWNING, it should be removed from the > map. If checkpoint is marked as inapplicable for a certain group, the whole > group should be removed from the map. > 2) On checkpoint history cleanup. For every (grpId, partId), previous > earliest checkpoint should be changed with setIfGreater to new earliest > checkpoint. > We should also extract WAL pointer reservation and filtering small partitions > from reserveHistoryForExchange(), but this shouldn't be a problem. > Another point for optimization: searchPartitionCounter() and > searchCheckpointEntry() are executed for each (grpId, partId). That means > we'll perform O(number of partitions) linear lookups in history. This should > be optimized as well: we can perform one lookup for all (grpId, partId) > pairs. This is especially critical for reserveHistoryForPreloading() method > complexity: it's executed from exchange thread. > Memory overhead of storing described map on heap is insignificant. Its size > isn't greater than size of map returned from reserveHistoryForExchange(). > Described fix should be much simpler than IGNITE-12429. > P.S. Possibly, instead of storing map, we can keep earliestCheckpoint right > in GridDhtLocalPartition. It may simplify implementation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-13052) Calculate result of reserveHistoryForExchange in advance
[ https://issues.apache.org/jira/browse/IGNITE-13052?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129551#comment-17129551 ] Vladislav Pyatkov commented on IGNITE-13052: This is an appropriated PR: [https://github.com/apache/ignite/pull/7911] > Calculate result of reserveHistoryForExchange in advance > > > Key: IGNITE-13052 > URL: https://issues.apache.org/jira/browse/IGNITE-13052 > Project: Ignite > Issue Type: Improvement >Reporter: Ivan Rakov >Assignee: Vladislav Pyatkov >Priority: Major > Original Estimate: 80h > Remaining Estimate: 80h > > Method reserveHistoryForExchange() is called on every partition map exchange. > It's an expensive call: it requires iteration over the whole checkpoint > history with possible retrieve of GroupState from WAL (it's stored on heap > with SoftReference). On some deployments this operation can take several > minutes. > The idea of optimization is to calculate its result only on first PME > (ideally, even before first PME, on recovery stage), keep resulting map > (grpId, partId -> earlisetCheckpoint) on heap and update it if necessary. > From the first glance, the map should be updated: > 1) On checkpoint. If a new partition appears on local node, it should be > registered in the map with current checkpoint. If a partition is evicted from > local node, or changes its state to non-OWNING, it should be removed from the > map. If checkpoint is marked as inapplicable for a certain group, the whole > group should be removed from the map. > 2) On checkpoint history cleanup. For every (grpId, partId), previous > earliest checkpoint should be changed with setIfGreater to new earliest > checkpoint. > We should also extract WAL pointer reservation and filtering small partitions > from reserveHistoryForExchange(), but this shouldn't be a problem. > Another point for optimization: searchPartitionCounter() and > searchCheckpointEntry() are executed for each (grpId, partId). That means > we'll perform O(number of partitions) linear lookups in history. This should > be optimized as well: we can perform one lookup for all (grpId, partId) > pairs. This is especially critical for reserveHistoryForPreloading() method > complexity: it's executed from exchange thread. > Memory overhead of storing described map on heap is insignificant. Its size > isn't greater than size of map returned from reserveHistoryForExchange(). > Described fix should be much simpler than IGNITE-12429. > P.S. Possibly, instead of storing map, we can keep earliestCheckpoint right > in GridDhtLocalPartition. It may simplify implementation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-13052) Calculate result of reserveHistoryForExchange in advance
[ https://issues.apache.org/jira/browse/IGNITE-13052?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129127#comment-17129127 ] Vladislav Pyatkov commented on IGNITE-13052: This test failed as before. We cannot view this because master does not run on TC daily. [~irakov] Could please review change? > Calculate result of reserveHistoryForExchange in advance > > > Key: IGNITE-13052 > URL: https://issues.apache.org/jira/browse/IGNITE-13052 > Project: Ignite > Issue Type: Improvement >Reporter: Ivan Rakov >Assignee: Vladislav Pyatkov >Priority: Major > Original Estimate: 80h > Remaining Estimate: 80h > > Method reserveHistoryForExchange() is called on every partition map exchange. > It's an expensive call: it requires iteration over the whole checkpoint > history with possible retrieve of GroupState from WAL (it's stored on heap > with SoftReference). On some deployments this operation can take several > minutes. > The idea of optimization is to calculate its result only on first PME > (ideally, even before first PME, on recovery stage), keep resulting map > (grpId, partId -> earlisetCheckpoint) on heap and update it if necessary. > From the first glance, the map should be updated: > 1) On checkpoint. If a new partition appears on local node, it should be > registered in the map with current checkpoint. If a partition is evicted from > local node, or changes its state to non-OWNING, it should be removed from the > map. If checkpoint is marked as inapplicable for a certain group, the whole > group should be removed from the map. > 2) On checkpoint history cleanup. For every (grpId, partId), previous > earliest checkpoint should be changed with setIfGreater to new earliest > checkpoint. > We should also extract WAL pointer reservation and filtering small partitions > from reserveHistoryForExchange(), but this shouldn't be a problem. > Another point for optimization: searchPartitionCounter() and > searchCheckpointEntry() are executed for each (grpId, partId). That means > we'll perform O(number of partitions) linear lookups in history. This should > be optimized as well: we can perform one lookup for all (grpId, partId) > pairs. This is especially critical for reserveHistoryForPreloading() method > complexity: it's executed from exchange thread. > Memory overhead of storing described map on heap is insignificant. Its size > isn't greater than size of map returned from reserveHistoryForExchange(). > Described fix should be much simpler than IGNITE-12429. > P.S. Possibly, instead of storing map, we can keep earliestCheckpoint right > in GridDhtLocalPartition. It may simplify implementation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IGNITE-13052) Calculate result of reserveHistoryForExchange in advance
[ https://issues.apache.org/jira/browse/IGNITE-13052?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17128968#comment-17128968 ] Ignite TC Bot commented on IGNITE-13052: {panel:title=Branch: [pull/7911/head] Base: [master] : Possible Blockers (1)|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1} {color:#d04437}Cache 5{color} [[tests 1|https://ci.ignite.apache.org/viewLog.html?buildId=5374895]] * IgniteCacheWithIndexingTestSuite: CacheRegisterMetadataLocallyTest.testClientFindsValueByAffinityKeyStaticCacheWithoutExtraRequest - Test has low fail rate in base branch 0,0% and is not flaky {panel} [TeamCity *--> Run :: All* Results|https://ci.ignite.apache.org/viewLog.html?buildId=5373489&buildTypeId=IgniteTests24Java8_RunAll] > Calculate result of reserveHistoryForExchange in advance > > > Key: IGNITE-13052 > URL: https://issues.apache.org/jira/browse/IGNITE-13052 > Project: Ignite > Issue Type: Improvement >Reporter: Ivan Rakov >Assignee: Vladislav Pyatkov >Priority: Major > Original Estimate: 80h > Remaining Estimate: 80h > > Method reserveHistoryForExchange() is called on every partition map exchange. > It's an expensive call: it requires iteration over the whole checkpoint > history with possible retrieve of GroupState from WAL (it's stored on heap > with SoftReference). On some deployments this operation can take several > minutes. > The idea of optimization is to calculate its result only on first PME > (ideally, even before first PME, on recovery stage), keep resulting map > (grpId, partId -> earlisetCheckpoint) on heap and update it if necessary. > From the first glance, the map should be updated: > 1) On checkpoint. If a new partition appears on local node, it should be > registered in the map with current checkpoint. If a partition is evicted from > local node, or changes its state to non-OWNING, it should be removed from the > map. If checkpoint is marked as inapplicable for a certain group, the whole > group should be removed from the map. > 2) On checkpoint history cleanup. For every (grpId, partId), previous > earliest checkpoint should be changed with setIfGreater to new earliest > checkpoint. > We should also extract WAL pointer reservation and filtering small partitions > from reserveHistoryForExchange(), but this shouldn't be a problem. > Another point for optimization: searchPartitionCounter() and > searchCheckpointEntry() are executed for each (grpId, partId). That means > we'll perform O(number of partitions) linear lookups in history. This should > be optimized as well: we can perform one lookup for all (grpId, partId) > pairs. This is especially critical for reserveHistoryForPreloading() method > complexity: it's executed from exchange thread. > Memory overhead of storing described map on heap is insignificant. Its size > isn't greater than size of map returned from reserveHistoryForExchange(). > Described fix should be much simpler than IGNITE-12429. > P.S. Possibly, instead of storing map, we can keep earliestCheckpoint right > in GridDhtLocalPartition. It may simplify implementation. -- This message was sent by Atlassian Jira (v8.3.4#803005)