-
Notifications
You must be signed in to change notification settings - Fork 0
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
Ignite 4756 #3
base: master
Are you sure you want to change the base?
Ignite 4756 #3
Conversation
@@ -83,7 +84,7 @@ | |||
private int parts; | |||
|
|||
/** Mask to use in calculation when partitions count is power of 2. */ | |||
private int mask = -1; | |||
private transient int mask = -1; |
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.
а зачем transient?
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.
Убрал
@@ -493,9 +496,91 @@ private static long hash(int key0, int key1) { | |||
assignments.add(partAssignment); | |||
} | |||
|
|||
List<List<Integer>> dist = freqDistribution(assignments, nodes); | |||
printDistribution(dist); |
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.
отступы
|
||
for (int j = 0; j < backups; ++j) { | ||
if (nodeMaps.get(j).get(node) == null) | ||
byBackups.add(0); |
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.
Вообще почему у тебя отступы по 8 пробелов? 4 пробела во всем проекте
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.
Не понял, по 4-ре вроде везде
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.
присмотрись но тут 8))
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.
восемь да, это жен цикл в цикле
|
||
log.info(""); | ||
} | ||
|
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.
убрать строку
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.
какую?
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.
под которой этот коммент
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.
убрал
@@ -351,6 +351,24 @@ private double chiSquare(List<List<Integer>> byNodes, int parts, double goldenNo | |||
/** | |||
* @throws IOException On error. | |||
*/ | |||
public void testImprovedLog() throws IOException { | |||
int[] nodesCnts1 = {4}; |
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.
nodesCnts
public void testImprovedLog() throws IOException { | ||
int[] nodesCnts1 = {4}; | ||
|
||
AffinityFunction aff3 = new RendezvousAffinityFunction(true, 1024); |
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.
aff
|
||
AffinityFunction aff3 = new RendezvousAffinityFunction(true, 1024); | ||
|
||
for (int nodesCnt1 : nodesCnts1) { |
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.
nodesCnt
List<ClusterNode> nodes0 = createBaseNodes(nodesCnt1); | ||
|
||
assignPartitions(aff3, nodes0, null, 2, 0).get2(); | ||
List<List<ClusterNode>> lst0 = assignPartitions(aff3, nodes0, null, 2, 1).get2(); |
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.
lst
assignPartitions(aff3, nodes0, null, 2, 0).get2(); | ||
List<List<ClusterNode>> lst0 = assignPartitions(aff3, nodes0, null, 2, 1).get2(); | ||
|
||
List<List<Integer>> dist0 = freqDistribution(lst0, nodes0); |
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.
dist
for (int nodesCnt : nodesCnts) { | ||
List<ClusterNode> nodes0 = createBaseNodes(nodesCnt); | ||
|
||
assignPartitions(aff, nodes0, null, 2, 0).get2(); |
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.
nodes
* @return Frequency distribution: counts of partitions on node. | ||
*/ | ||
private static List<List<Integer>> freqDistribution(List<List<ClusterNode>> lst, Collection<ClusterNode> nodes) { | ||
List<Map<ClusterNode, AtomicInteger>> nodeMaps = new ArrayList<>(); |
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.
1 пробел 1 символ = посчитай что у тебя))
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.
не понимаю, напиши плз как должно быть
log.info("#### NODES:" + nodes + " nodes\n"); | ||
|
||
for (List<Integer> byNode : byNodes) { | ||
|
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.
убрать строку
|
||
nr = 0; | ||
|
||
for (int w : byNode) { |
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.
а почему переменная называется w
может лучше weight
смотря что у тебя оно обозначает
int nr = 0; | ||
int node = 0; | ||
|
||
log.info("#### NODES:" + nodes + " nodes\n"); |
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.
log.info("#### NODES:" + nodes + "\n");
зачем 2 раза повторять одно и тоже слово
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.
Исправлено
nr++; | ||
} | ||
|
||
log.info(""); |
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.
а вот это зачем?
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.
убрал
import org.apache.ignite.resources.LoggerResource; | ||
import org.jetbrains.annotations.Nullable; | ||
//import org.apache.ignite.logger.log4j.Log4JLogger; |
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.
а это почему осталась?
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.
убрал
Improved Java Doc and Code Style.
|
||
/** | ||
* @param nodesParts Frequency distribution. | ||
* @param localNodeID |
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.
Почему нет описание переменной и дальше тоже
/** | ||
* Gets node name. | ||
* | ||
* @return Cache name. |
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.
а тут id
Improved Java Doc, Code Style and Abbreviation.
Improved Java Doc, Code Style and Abbreviation.
@@ -280,21 +282,26 @@ public void onReconnected() { | |||
|
|||
List<List<ClusterNode>> assignment; | |||
|
|||
GridAffinityFunctionContextImpl afCtxt = new GridAffinityFunctionContextImpl(sorted, prevAssignment, |
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.
Давай вернем как было в мастере.
if (prevAssignment != null && discoEvt != null) { | ||
boolean affNode = CU.affinityNode(discoEvt.eventNode(), nodeFilter); | ||
|
||
if (!affNode) | ||
assignment = prevAssignment; | ||
else | ||
assignment = aff.assignPartitions(new GridAffinityFunctionContextImpl(sorted, prevAssignment, | ||
discoEvt, topVer, backups)); | ||
assignment = aff.assignPartitions(afCtxt); |
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.
Тут тоже
} | ||
else | ||
assignment = aff.assignPartitions(new GridAffinityFunctionContextImpl(sorted, prevAssignment, discoEvt, | ||
topVer, backups)); | ||
assignment = aff.assignPartitions(afCtxt); |
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.
и тут
@@ -17,10 +17,6 @@ | |||
|
|||
package org.apache.ignite.cache.affinity.rendezvous; | |||
|
|||
import java.io.Externalizable; |
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.
Почему это есть в мастере а у тебя удалено? Нужно вернуть.
* @param nodes Topology. | ||
* @return Frequency distribution: counts of partitions on node. | ||
*/ | ||
private static List<List<Integer>> freqDistribution(List<List<ClusterNode>> partitionsByNodes, |
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.
почему статик?
@@ -349,14 +352,25 @@ private double chiSquare(List<List<Integer>> byNodes, int parts, double goldenNo | |||
} | |||
|
|||
/** | |||
* @throws Exception If failed. | |||
*/ | |||
public void testImprovedLog() throws Exception { |
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.
Однозначно написать тест проверки распределения.
/** | ||
* @return Ignite Logger. | ||
*/ | ||
private IgniteLogger initializeIgniteLogger() { |
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.
убрать
* @param log Logger instance. | ||
* @param ignite Ignite instance. | ||
*/ | ||
private AffinityFunction initializeAffinityFunction(boolean isOld, int parts, boolean exclNeighbors, |
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.
убрать
// TODO choose another affinity function to compare. | ||
AffinityFunction aff1 = new RendezvousAffinityFunction(true, 256); | ||
AffinityFunction aff0 = initializeAffinityFunction(false, 256, true, log, null); |
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.
вернуть
@@ -637,6 +642,8 @@ | |||
public static final String IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT = | |||
"IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT"; | |||
|
|||
|
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.
зачем 2 пустых строки?
@@ -295,6 +298,8 @@ public void onReconnected() { | |||
|
|||
assert assignment != null; | |||
|
|||
printDistribution(assignment, sorted, cacheOrGrpName, ctx.localNodeId().toString(), assignment.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.
ты передаешь assignment, зачем передавать assignment.size, ты же сможешь получить это
String ignitePartDistribution = System.getProperty(IgniteSystemProperties.IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD); | ||
|
||
if (Boolean.valueOf(ignitePartDistribution) || ignitePartDistribution == null) { | ||
printDistribution(assignment, sorted, cacheOrGrpName, ctx.localNodeId().toString(), assignment.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.
тебе не нужно передавать assignment.size() он у тебя уже есть в assignment
@@ -414,6 +414,11 @@ | |||
public static final String IGNITE_CACHE_CLIENT = "IGNITE_CACHE_CLIENT"; | |||
|
|||
/** | |||
* Property controlling distribution calculation flag. | |||
*/ | |||
public static final String IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD = "IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD"; |
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.
эта опция не флаг, прочитай внимательнее
Add system property IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD with default value 0.1 to print warn message only when nodes count differs more then threshold;
public class RendezvousAffinityFunctionCalculateDistributionSelfTest extends AbstractAffinityFunctionSelfTest { | ||
/** Ignite. */ | ||
private static Ignite ignite; | ||
|
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.
Лучше переопределить метод afterTests что бы в нем останавливать ноды.
* @throws Exception If failed. | ||
*/ | ||
public void testDistributionCalculationEnabled() throws Exception { | ||
System.setProperty(IgniteSystemProperties.IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD, String.valueOf(true)); |
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.
Как уже выше писал, здесь не булевое значение.
No description provided.