[
https://issues.apache.org/jira/browse/IGNITE-14228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17302330#comment-17302330
]
Anton Vinogradov edited comment on IGNITE-14228 at 3/16/21, 9:07 AM:
---------------------------------------------------------------------
[~cyberdemon], [~nizhikov] thanks for your contribution, but I'd like to
mention some tips related to the merged code.
1) Some code does nothing but enlarges PR/file size.
For example,
{noformat}
if (n >= MAX_STREAMER_DATA_SIZE) {
n = 0;
stmr.flush();
}
{noformat}
provides no real value, but takes 8 lines with const definition.
2) Some code makes some ... overcomplicated magic, at least for the reader who
faces it the first time.
{{int logStreamedEntriesQuant = (int)Math.pow(10, (int)Math.log10(entryCnt) -
1);}}
Do we really need such calculations for logging? Why not just {{if i % z == 0
...}} ?
3) In case you have custom await logic you must make it strict
{noformat}
app, app.nodes[0], "Marking as initialized") - get_event_time(
app, app.nodes[0], "Data generation started")).total_seconds()
{noformat}
Here, you invented the {{Data generation started}} event, and it will be much
better if you'd also invent the {{finished}} event, this would make core
refactoring friendly.
4) Exception should never be hidden
{noformat}
for node in ignite.nodes:
try:
rebalance_start_time = get_event_time(
ignite, node,
"Starting rebalance routine \\[test-cache-",
timeout=timeout)
except TimeoutError:
continue
else:
return {"node": node, "time": rebalance_start_time}
{noformat}
Once you got an Exception you must handle it, not pass.
5) Your code should always be as simple as possible,
for example, following
{noformat}
for cache_idx in range(cache_count):
rebalance_complete_times.append(get_event_time(
ignite,
node if node else ignite.nodes[0],
"Completed rebalance future: RebalanceFuture \\[%s
\\[grp=test-cache-%d" %
("state=STARTED, grp=CacheGroupContext", cache_idx + 1),
timeout=timeout))
return max(rebalance_complete_times)
{noformat}
can be replaced with just one line
{noformat}
get_event_time(..., "Completed rebalance (final)", ...)
{noformat}
6) Never build a Wunderwaffe
At the following code, you never need {{node}} and {{time}} as dictionary
indices,
{noformat}
start_node_and_time = await_rebalance_start(ignite)
complete_time = await_rebalance_complete(ignite, start_node_and_time["node"],
cache_count)
return {"Rebalanced in (sec)": (complete_time -
start_node_and_time["time"]).total_seconds(),
{noformat}
all you need here is to return tupple instead.
was (Author: av):
[~cyberdemon], [~nizhikov] thanks for your contribution, but I'd like to
mention some tips related to the merged code.
1) Some code does nothing but enlarges PR/file size.
For example,
{noformat}
if (n >= MAX_STREAMER_DATA_SIZE) {
n = 0;
stmr.flush();
}
{noformat}
provides no real value, but takes 8 lines with const definition.
2) Some code makes some ... overcomplicated magic, at least for the reader who
faces it the first time.
{{int logStreamedEntriesQuant = (int)Math.pow(10, (int)Math.log10(entryCnt) -
1);}}
Do we really need such calculations for logging? Why not just {{if i % z == 0
...}} ?
3) In case you have custom await logic you must make it strict
{noformat}
app, app.nodes[0], "Marking as initialized") - get_event_time(
app, app.nodes[0], "Data generation started")).total_seconds()
{noformat}
Here, you invented the {{Data generation started}} event, and it will be much
better if you'd also invent the {{finished}} event, this would make core
refactoring friendly.
4) Exception should never be hidden
{noformat}
for node in ignite.nodes:
try:
rebalance_start_time = get_event_time(
ignite, node,
"Starting rebalance routine \\[test-cache-",
timeout=timeout)
except TimeoutError:
continue
else:
return {"node": node, "time": rebalance_start_time}
{noformat}
Once you got an Exception you must handle it, not pass.
5) Your code should always be as simple as possible,
for example, following
{noformat}
for cache_idx in range(cache_count):
rebalance_complete_times.append(get_event_time(
ignite,
node if node else ignite.nodes[0],
"Completed rebalance future: RebalanceFuture \\[%s
\\[grp=test-cache-%d" %
("state=STARTED, grp=CacheGroupContext", cache_idx + 1),
timeout=timeout))
return max(rebalance_complete_times)
{noformat}
can be replaced with just one line
{noformat}
get_event_time(..., "Completed rebalance (final)", ...)
{noformat}
6) Newer build a Wunderwaffe
At the following code, you never need {{node}} and {{time}} as dictionary
indices,
{noformat}
start_node_and_time = await_rebalance_start(ignite)
complete_time = await_rebalance_complete(ignite, start_node_and_time["node"],
cache_count)
return {"Rebalanced in (sec)": (complete_time -
start_node_and_time["time"]).total_seconds(),
{noformat}
all you need here is to return tupple instead.
> Ducktape test of rebalance for in-memory mode
> ---------------------------------------------
>
> Key: IGNITE-14228
> URL: https://issues.apache.org/jira/browse/IGNITE-14228
> Project: Ignite
> Issue Type: Sub-task
> Reporter: Dmitriy Sorokin
> Assignee: Dmitriy Sorokin
> Priority: Major
> Time Spent: 5.5h
> Remaining Estimate: 0h
>
> This test should check the rebalance on in-memory caches in basic scenario:
> rebalance triggered by two event types: node join and node left the topology.
> The test should be able to run on large amounts of data enough to ensure
> rebalancing time about of several minutes.
> Test parameters:
> # Initial node count (derived from test context);
> # Cache count - the count of caches to create and data preload;
> # Cache entry count - the count of entries per cache to preload;
> # Cache entry size - the size of each cache entry in bytes;
> # Rebalance thread pool size - the value for
> {{IgniteConfiguration#rebalanceThreadPoolSize}} property (see [configuring
> rebalance thread pool
> title|https://ignite.apache.org/docs/latest/data-rebalancing#configuring-rebalance-thread-pool]),
> expected that greater value makes rebalance faster.
> # Rebalance batch size - the value for
> {{IgniteConfiguration#rebalanceBatchSize}} property (see [other rebalance
> properties|https://ignite.apache.org/docs/latest/data-rebalancing#other-properties]),
> expected that greater value makes rebalance faster on large data or
> [throttling|https://ignite.apache.org/docs/latest/data-rebalancing#throttling]
> enabled (rebalanceThrottling > 0).
> # Rebalance throttle - the value for
> {{IgniteConfiguration#rebalanceThrottle}} property (see [rebalance message
> throttling|https://ignite.apache.org/docs/latest/data-rebalancing#throttling],
> [other rebalance
> properties|https://ignite.apache.org/docs/latest/data-rebalancing#other-properties]),
> 0 - throttling disabled, greater value makes rebalance slower.
> Test result: time taken to rebalance process.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
