From 36edf02f244a744945d96f6bf034d26335d63fc5 Mon Sep 17 00:00:00 2001 From: Karthick Duraisamy Soundararaj Date: Wed, 20 Jul 2016 11:24:25 -0700 Subject: [PATCH 1/2] Improve few existing comments, add more comments for existing functions, delete few unnecessary/irrelevant methods --- storm/src/main/storm/mesos/MesosNimbus.java | 4 ---- .../storm/mesos/resources/AggregatedOffers.java | 16 ++++------------ .../storm/mesos/resources/RangeResource.java | 6 +++++- .../src/main/storm/mesos/resources/Resource.java | 2 ++ .../storm/mesos/resources/ResourceEntries.java | 5 +++++ .../storm/mesos/resources/ScalarResource.java | 9 +-------- 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/storm/src/main/storm/mesos/MesosNimbus.java b/storm/src/main/storm/mesos/MesosNimbus.java index 5aa5be7cf..b32ee967a 100644 --- a/storm/src/main/storm/mesos/MesosNimbus.java +++ b/storm/src/main/storm/mesos/MesosNimbus.java @@ -384,10 +384,6 @@ public Collection allSlotsAvailableForScheduling( } } - private String getLogViewerConfig() { - return String.format(" -c %s=true", MesosCommon.AUTO_START_LOGVIEWER_CONF); - } - /** * This method is invoked after IScheduler.schedule assigns the worker slots to the topologies that need assignments * diff --git a/storm/src/main/storm/mesos/resources/AggregatedOffers.java b/storm/src/main/storm/mesos/resources/AggregatedOffers.java index 645dcae77..153d0e246 100644 --- a/storm/src/main/storm/mesos/resources/AggregatedOffers.java +++ b/storm/src/main/storm/mesos/resources/AggregatedOffers.java @@ -47,11 +47,6 @@ private void initializeAvailableResources() { availableResources.put(ResourceType.PORTS, new RangeResource(ResourceType.PORTS)); } - public AggregatedOffers(String hostName) { - this.hostName = hostName; - initializeAvailableResources(); - } - public AggregatedOffers(Protos.Offer offer) { initializeAvailableResources(); this.slaveID = offer.getSlaveId(); @@ -64,7 +59,7 @@ public String getHostName() { } public void add(Protos.Offer offer) { - // We are unable to aggregate offers if they are from different workers + // We are unable to aggregate offers if they are from different mesos slaves/workers/agents assert offer.getSlaveId().equals(slaveID) && offer.getHostname().equals(hostName); offerList.add(offer); @@ -103,6 +98,7 @@ public boolean isAvailable(ResourceType resourceType, ResourceEntry resource) return availableResources.get(resourceType).isAvailable(resource); } + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public boolean isAvailable(ResourceType resourceType, ReservationType reservationType, ResourceEntry resource) { return availableResources.get(resourceType).isAvailable(resource, reservationType); } @@ -111,6 +107,7 @@ public List getAllAvailableResources(ResourceType r return availableResources.get(resourceType).getAllAvailableResources(); } + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public List getAllAvailableResources(ResourceType resourceType, ReservationType reservationType) { return availableResources.get(resourceType).getAllAvailableResources(reservationType); } @@ -128,6 +125,7 @@ public List reserveAndGet(ResourceType resourceType, ResourceEntr return new ArrayList<>(); } + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public List reserveAndGet(ResourceType resourceType, ReservationType reservationType, ResourceEntry resource) throws ResourceNotAvailableException { if (availableResources.get(resourceType).isAvailable(resource, reservationType)) { @@ -136,10 +134,6 @@ public List reserveAndGet(ResourceType resourceType, ReservationT return new ArrayList<>(); } - public List getOfferList() { - return offerList; - } - public List getOfferIDList() { List offerIDList = new ArrayList<>(); for (Protos.Offer offer: offerList) { @@ -162,7 +156,6 @@ public String toString() { public boolean isFit(Map mesosStormConf, TopologyDetails topologyDetails, boolean supervisorExists) { - double requestedWorkerCpu = MesosCommon.topologyWorkerCpu(mesosStormConf, topologyDetails); double requestedWorkerMem = MesosCommon.topologyWorkerMem(mesosStormConf, topologyDetails); @@ -175,7 +168,6 @@ public boolean isFit(Map mesosStormConf, TopologyDetails topologyDetails, boolea } public boolean isFit(Map mesosStormConf, TopologyDetails topologyDetails, Long port, boolean supervisorExists) { - double requestedWorkerCpu = MesosCommon.topologyWorkerCpu(mesosStormConf, topologyDetails); double requestedWorkerMem = MesosCommon.topologyWorkerMem(mesosStormConf, topologyDetails); diff --git a/storm/src/main/storm/mesos/resources/RangeResource.java b/storm/src/main/storm/mesos/resources/RangeResource.java index aada9589d..c0b588521 100644 --- a/storm/src/main/storm/mesos/resources/RangeResource.java +++ b/storm/src/main/storm/mesos/resources/RangeResource.java @@ -108,11 +108,13 @@ public List removeAndGet(RangeResourceEntry rangeResourceEntry) t /** + * Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + * * Remove/Reserve range from available ranges. * {@param rangeResourceEntry} range resource to removeAndGet * {@param reservationType} reservation type of resource that needs to be removed. If the resource represented by rangeResourceEntry * of the reservation type specified by this parameter is not available, then {@link ResourceNotAvailableException} - * is thrown + * is thrown. */ @Override public List removeAndGet(RangeResourceEntry rangeResourceEntry, ReservationType reservationType) throws ResourceNotAvailableException { @@ -129,6 +131,8 @@ public List removeAndGet(RangeResourceEntry rangeResourceEntry, R } /** + * Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + * * Remove/Reserve range from available ranges * {@param rangeResourceEntry} range resource to removeAndGet * {@param reservationTypeComparator} comparator like {@link storm.mesos.resources.DefaultReservationTypeComparator} diff --git a/storm/src/main/storm/mesos/resources/Resource.java b/storm/src/main/storm/mesos/resources/Resource.java index 2e5931570..d164f0c41 100644 --- a/storm/src/main/storm/mesos/resources/Resource.java +++ b/storm/src/main/storm/mesos/resources/Resource.java @@ -38,8 +38,10 @@ public interface Resource> { public List removeAndGet(T resourceEntry) throws ResourceNotAvailableException; + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public List removeAndGet(T value, ReservationType reservationType) throws ResourceNotAvailableException; + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public List removeAndGet(T value, Comparator reservationTypeComparator) throws ResourceNotAvailableException; } diff --git a/storm/src/main/storm/mesos/resources/ResourceEntries.java b/storm/src/main/storm/mesos/resources/ResourceEntries.java index f71473451..72b6bac7a 100644 --- a/storm/src/main/storm/mesos/resources/ResourceEntries.java +++ b/storm/src/main/storm/mesos/resources/ResourceEntries.java @@ -83,6 +83,8 @@ public ReservationType getReservationType() { } /** + * Unused Method - Exists for the sake of completeness in terms of implementing ResourceEntry. + * * Lets say, we have a range [u,v]. Using this add function, we can expand the range to [w,x] if and * only if one of the following conditions are satisfied * `w < u` @@ -107,6 +109,9 @@ public RangeResourceEntry add(ResourceEntry resourceEntry) { return this; } + /** + * Unused Method - Exists for the sake of completeness in terms of implementing ResourceEntry. + */ public RangeResourceEntry remove(ResourceEntry resourceEntry) { RangeResourceEntry rangeResourceEntry = (RangeResourceEntry) resourceEntry; if (this.begin < rangeResourceEntry.getBegin()) { diff --git a/storm/src/main/storm/mesos/resources/ScalarResource.java b/storm/src/main/storm/mesos/resources/ScalarResource.java index 3ca357734..85836815b 100644 --- a/storm/src/main/storm/mesos/resources/ScalarResource.java +++ b/storm/src/main/storm/mesos/resources/ScalarResource.java @@ -52,6 +52,7 @@ public boolean isAvailable(ScalarResourceEntry scalarResourceEntry, ReservationT return (availableResourcesByReservationType.get(reservationType).getValue() >= scalarResourceEntry.getValue()); } + // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 public Double getTotalAvailableResource(ReservationType reservationType) { return availableResourcesByReservationType.get(reservationType).getValue(); } @@ -82,14 +83,6 @@ public List removeAndGet(ScalarResourceEntry scalarResourceEntry) return removeAndGet(scalarResourceEntry, availableResourcesByReservationType.keySet()); } - public List reserveScalarResource(ResourceType resourceType, ScalarResourceEntry requiredValue) throws ResourceNotAvailableException { - if (totalAvailableResource < requiredValue.getValue()) { - throw new ResourceNotAvailableException(String.format("resourceType: {} is not available. Requested {} Available {}", - resourceType, requiredValue, totalAvailableResource)); - } - return removeAndGet(requiredValue); - } - /** * Removes/Reserves scalar resource from available resources. * {@param scalarResourceEntry} amount of scalar resource to removeAndGet/decrement From 29831fdd52271d40c72ff8c6ce2a25c5ccdf01bd Mon Sep 17 00:00:00 2001 From: Karthick Duraisamy Soundararaj Date: Wed, 20 Jul 2016 12:33:49 -0700 Subject: [PATCH 2/2] Modifying comments as per Jessica's comment --- .../mesos/resources/AggregatedOffers.java | 18 +++++++++++++++--- .../storm/mesos/resources/RangeResource.java | 8 ++++++-- .../main/storm/mesos/resources/Resource.java | 12 ++++++++++-- .../storm/mesos/resources/ScalarResource.java | 6 +++++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/storm/src/main/storm/mesos/resources/AggregatedOffers.java b/storm/src/main/storm/mesos/resources/AggregatedOffers.java index 153d0e246..50dbdcfbe 100644 --- a/storm/src/main/storm/mesos/resources/AggregatedOffers.java +++ b/storm/src/main/storm/mesos/resources/AggregatedOffers.java @@ -98,7 +98,11 @@ public boolean isAvailable(ResourceType resourceType, ResourceEntry resource) return availableResources.get(resourceType).isAvailable(resource); } - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public boolean isAvailable(ResourceType resourceType, ReservationType reservationType, ResourceEntry resource) { return availableResources.get(resourceType).isAvailable(resource, reservationType); } @@ -107,7 +111,11 @@ public List getAllAvailableResources(ResourceType r return availableResources.get(resourceType).getAllAvailableResources(); } - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public List getAllAvailableResources(ResourceType resourceType, ReservationType reservationType) { return availableResources.get(resourceType).getAllAvailableResources(reservationType); } @@ -125,7 +133,11 @@ public List reserveAndGet(ResourceType resourceType, ResourceEntr return new ArrayList<>(); } - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public List reserveAndGet(ResourceType resourceType, ReservationType reservationType, ResourceEntry resource) throws ResourceNotAvailableException { if (availableResources.get(resourceType).isAvailable(resource, reservationType)) { diff --git a/storm/src/main/storm/mesos/resources/RangeResource.java b/storm/src/main/storm/mesos/resources/RangeResource.java index c0b588521..685893123 100644 --- a/storm/src/main/storm/mesos/resources/RangeResource.java +++ b/storm/src/main/storm/mesos/resources/RangeResource.java @@ -108,7 +108,9 @@ public List removeAndGet(RangeResourceEntry rangeResourceEntry) t /** - * Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 * * Remove/Reserve range from available ranges. * {@param rangeResourceEntry} range resource to removeAndGet @@ -131,7 +133,9 @@ public List removeAndGet(RangeResourceEntry rangeResourceEntry, R } /** - * Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 * * Remove/Reserve range from available ranges * {@param rangeResourceEntry} range resource to removeAndGet diff --git a/storm/src/main/storm/mesos/resources/Resource.java b/storm/src/main/storm/mesos/resources/Resource.java index d164f0c41..d9073f27a 100644 --- a/storm/src/main/storm/mesos/resources/Resource.java +++ b/storm/src/main/storm/mesos/resources/Resource.java @@ -38,10 +38,18 @@ public interface Resource> { public List removeAndGet(T resourceEntry) throws ResourceNotAvailableException; - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public List removeAndGet(T value, ReservationType reservationType) throws ResourceNotAvailableException; - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public List removeAndGet(T value, Comparator reservationTypeComparator) throws ResourceNotAvailableException; } diff --git a/storm/src/main/storm/mesos/resources/ScalarResource.java b/storm/src/main/storm/mesos/resources/ScalarResource.java index 85836815b..ee246f1a7 100644 --- a/storm/src/main/storm/mesos/resources/ScalarResource.java +++ b/storm/src/main/storm/mesos/resources/ScalarResource.java @@ -52,7 +52,11 @@ public boolean isAvailable(ScalarResourceEntry scalarResourceEntry, ReservationT return (availableResourcesByReservationType.get(reservationType).getValue() >= scalarResourceEntry.getValue()); } - // Unused Method - Exists for the reason described in https://github.com/mesos/storm/pull/146#issuecomment-225496075 + /** + * Unused Method - Exists for the purpose of facilitating support of reservations. + * TODO: Support reservations (https://github.com/mesos/storm/issues/148) + * For more information about why this unused code exists, see discussion: https://github.com/mesos/storm/pull/146#issuecomment-225496075 + */ public Double getTotalAvailableResource(ReservationType reservationType) { return availableResourcesByReservationType.get(reservationType).getValue(); }