From dev-return-60428-archive-asf-public=cust-asf.ponee.io@storm.apache.org Sat Aug 10 17:38:28 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 6D937180607 for ; Sat, 10 Aug 2019 19:38:28 +0200 (CEST) Received: (qmail 29799 invoked by uid 500); 10 Aug 2019 17:38:25 -0000 Mailing-List: contact dev-help@storm.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@storm.apache.org Delivered-To: mailing list dev@storm.apache.org Received: (qmail 29610 invoked by uid 99); 10 Aug 2019 17:38:25 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 10 Aug 2019 17:38:25 +0000 From: GitBox To: dev@storm.apache.org Subject: [GitHub] [storm] d2r commented on a change in pull request #3050: STORM-3434: server: fix all checkstyle warnings Message-ID: <156545870491.32204.15556128076690037109.gitbox@gitbox.apache.org> Date: Sat, 10 Aug 2019 17:38:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit d2r commented on a change in pull request #3050: STORM-3434: server: fix all checkstyle warnings URL: https://github.com/apache/storm/pull/3050#discussion_r312710262 ########## File path: storm-server/src/main/java/org/apache/storm/scheduler/multitenant/Node.java ########## @@ -130,95 +133,110 @@ public static int countTotalSlotsAlive(Collection nodes) { } public String getId() { - return _nodeId; + return nodeId; } public boolean isAlive() { - return _isAlive; + return isAlive; } /** + * Get running topologies. * @return a collection of the topology ids currently running on this node */ public Collection getRunningTopologies() { - return _topIdToUsedSlots.keySet(); + return topIdToUsedSlots.keySet(); } public boolean isTotallyFree() { - return _topIdToUsedSlots.isEmpty(); + return topIdToUsedSlots.isEmpty(); } public int totalSlotsFree() { - return _freeSlots.size(); + return freeSlots.size(); } public int totalSlotsUsed() { int total = 0; - for (Set slots : _topIdToUsedSlots.values()) { + for (Set slots : topIdToUsedSlots.values()) { total += slots.size(); } return total; } - public int totalSlots() { - return totalSlotsFree() + totalSlotsUsed(); - } - public int totalSlotsUsed(String topId) { int total = 0; - Set slots = _topIdToUsedSlots.get(topId); + Set slots = topIdToUsedSlots.get(topId); if (slots != null) { total = slots.size(); } return total; } + public int totalSlots() { + return totalSlotsFree() + totalSlotsUsed(); + } + private void validateSlot(WorkerSlot ws) { - if (!_nodeId.equals(ws.getNodeId())) { - throw new IllegalArgumentException( - "Trying to add a slot to the wrong node " + ws + - " is not a part of " + _nodeId); + if (!nodeId.equals(ws.getNodeId())) { + throw new IllegalArgumentException("Trying to add a slot to the wrong node " + + ws + + " is not a part of " + + nodeId); } } private void addOrphanedSlot(WorkerSlot ws) { - if (_isAlive) { - throw new IllegalArgumentException("Orphaned Slots " + - "only are allowed on dead nodes."); + if (isAlive) { + throw new IllegalArgumentException("Orphaned Slots only are allowed on dead nodes."); } validateSlot(ws); - if (_freeSlots.contains(ws)) { + if (freeSlots.contains(ws)) { return; } - for (Set used : _topIdToUsedSlots.values()) { + for (Set used : topIdToUsedSlots.values()) { if (used.contains(ws)) { return; } } - _freeSlots.add(ws); + freeSlots.add(ws); } boolean assignInternal(WorkerSlot ws, String topId, boolean dontThrow) { validateSlot(ws); - if (!_freeSlots.remove(ws)) { - for (Entry> topologySetEntry : _topIdToUsedSlots.entrySet()) { + if (!freeSlots.remove(ws)) { + for (Entry> topologySetEntry : topIdToUsedSlots.entrySet()) { if (topologySetEntry.getValue().contains(ws)) { if (dontThrow) { - LOG.warn("Worker slot [" + ws + "] can't be assigned to " + topId + - ". Its already assigned to " + topologySetEntry.getKey() + "."); + LOG.warn("Worker slot [" + + ws + + "] can't be assigned to " + + topId + + ". Its already assigned to " + + topologySetEntry.getKey() + + "."); Review comment: This kind of enforced alignment will further encourage the use of format strings rather than string concatenation, because it is much less readable. I think this is a good thing. I am not suggesting we change all such occurrences to use format strings now—if we see a performance issue or if we want to change it for readability later I consider it out of scope for this pull request. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org With regards, Apache Git Services