From 1bdbbe78bb766e52fb8baf3ed53ddcec97eaf2f7 Mon Sep 17 00:00:00 2001 From: cdfive Date: Fri, 27 Mar 2020 23:36:14 +0800 Subject: [PATCH 1/5] Improve log info in SpiLoader, improve comment and test case --- .../csp/sentinel/log/jul/BaseJulLogger.java | 4 ++- .../slots/DefaultSlotChainBuilder.java | 18 ++++++++++- .../alibaba/csp/sentinel/util/SpiLoader.java | 32 +++++++++++++------ .../csp/sentinel/util/SpiLoaderTest.java | 27 +++++++++++++--- 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/jul/BaseJulLogger.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/jul/BaseJulLogger.java index e8a1fe6f30..0490efd299 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/jul/BaseJulLogger.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/log/jul/BaseJulLogger.java @@ -90,7 +90,9 @@ protected Handler makeLoggingHandler(String logName, Logger heliumRecordLog) { if (handler != null) { disableOtherHandlers(heliumRecordLog, handler); } - heliumRecordLog.setLevel(Level.ALL); + + // Set log level to INFO by default + heliumRecordLog.setLevel(Level.INFO); return handler; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java index 797206bb7b..33e04c8401 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java @@ -21,6 +21,7 @@ import com.alibaba.csp.sentinel.slotchain.ProcessorSlot; import com.alibaba.csp.sentinel.slotchain.ProcessorSlotChain; import com.alibaba.csp.sentinel.slotchain.SlotChainBuilder; +import com.alibaba.csp.sentinel.spi.SpiOrder; import com.alibaba.csp.sentinel.util.SpiLoader; import java.util.List; @@ -33,12 +34,27 @@ */ public class DefaultSlotChainBuilder implements SlotChainBuilder { + static { + List sortedSlotList = SpiLoader.loadPrototypeInstanceListSorted(ProcessorSlot.class); + for (ProcessorSlot slot : sortedSlotList) { + String slotClassCanonicalName = slot.getClass().getCanonicalName(); + int order; + if (slot.getClass().isAnnotationPresent(SpiOrder.class)) { + SpiOrder spiOrder = slot.getClass().getAnnotation(SpiOrder.class); + order = spiOrder.value(); + } else { + order = SpiOrder.LOWEST_PRECEDENCE; + } + RecordLog.info("[DefaultSlotChainBuilder]Found ProcessorSlot {} with order {}", slotClassCanonicalName, order); + } + } + @Override public ProcessorSlotChain build() { ProcessorSlotChain chain = new DefaultProcessorSlotChain(); // Note: the instances of ProcessorSlot should be different, since they are not stateless. - List sortedSlotList = SpiLoader.loadDifferentInstanceListSorted(ProcessorSlot.class); + List sortedSlotList = SpiLoader.loadPrototypeInstanceListSorted(ProcessorSlot.class); for (ProcessorSlot slot : sortedSlotList) { if (!(slot instanceof AbstractLinkedProcessorSlot)) { RecordLog.warn("The ProcessorSlot(" + slot.getClass().getCanonicalName() + ") is not an instance of AbstractLinkedProcessorSlot, can't be added into ProcessorSlotChain"); diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java index a677084ef0..fe7996ac20 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java @@ -34,6 +34,14 @@ public final class SpiLoader { private static final Map SERVICE_LOADER_MAP = new ConcurrentHashMap(); + /** + * Load the first-found specific SPI instance + * + * @param clazz class of the SPI interface + * @param SPI type + * @return the first specific SPI instance if exists, or else return null + * @since 1.7.0 + */ public static T loadFirstInstance(Class clazz) { AssertUtil.notNull(clazz, "SPI class cannot be null"); try { @@ -96,6 +104,8 @@ public static T loadFirstInstanceOrDefault(Class clazz, Class SPI type * @return the SPI instance with highest priority if exists, or else false @@ -132,6 +142,8 @@ public static T loadHighestPriorityInstance(Class clazz) { * Load and sorted SPI instance list. * Load the SPI instance list for provided SPI interface. * + * Note: each call return same instances. + * * @param clazz class of the SPI * @param SPI type * @return sorted SPI instance list @@ -149,6 +161,8 @@ public static List loadInstanceList(Class clazz) { List list = new ArrayList<>(); for (T spi : serviceLoader) { + RecordLog.info("[SpiLoader] Found {} SPI: {}", clazz.getSimpleName(), + spi.getClass().getCanonicalName()); list.add(spi); } return list; @@ -162,7 +176,7 @@ public static List loadInstanceList(Class clazz) { /** * Load the sorted SPI instance list for provided SPI interface. * - * Note: each call return new instances. + * Note: each call return same instances. * * @param clazz class of the SPI * @param SPI type @@ -184,8 +198,8 @@ public static List loadInstanceListSorted(Class clazz) { int order = SpiOrderResolver.resolveOrder(spi); // Since SPI is lazy initialized in ServiceLoader, we use online sort algorithm here. SpiOrderResolver.insertSorted(orderWrappers, spi, order); - RecordLog.info("[SpiLoader] Found {} SPI: {} with order " + order, clazz.getSimpleName(), - spi.getClass().getCanonicalName()); + RecordLog.info("[SpiLoader] Found {} SPI: {} with order {}", clazz.getSimpleName(), + spi.getClass().getCanonicalName(), order); } List list = new ArrayList<>(); for (int i = 0; i < orderWrappers.size(); i++) { @@ -200,16 +214,16 @@ public static List loadInstanceListSorted(Class clazz) { } /** - * Load the sorted and different SPI instance list for provided SPI interface. + * Load the sorted and prototype SPI instance list for provided SPI interface. * - * Note: each call return new instances. + * Note: each call return different instances, i.e. prototype instance, not singleton instance. * * @param clazz class of the SPI * @param SPI type * @return sorted and different SPI instance list * @since 1.7.2 */ - public static List loadDifferentInstanceListSorted(Class clazz) { + public static List loadPrototypeInstanceListSorted(Class clazz) { try { // Not use SERVICE_LOADER_MAP, to make sure the instances loaded are different. ServiceLoader serviceLoader = ServiceLoaderUtil.getServiceLoader(clazz); @@ -219,8 +233,8 @@ public static List loadDifferentInstanceListSorted(Class clazz) { int order = SpiOrderResolver.resolveOrder(spi); // Since SPI is lazy initialized in ServiceLoader, we use online sort algorithm here. SpiOrderResolver.insertSorted(orderWrappers, spi, order); - RecordLog.info("[SpiLoader] Found {0} SPI: {1} with order " + order, clazz.getSimpleName(), - spi.getClass().getCanonicalName()); + RecordLog.debug("[SpiLoader] Found {} SPI: {} with order {}", clazz.getSimpleName(), + spi.getClass().getCanonicalName(), order); } List list = new ArrayList<>(); for (int i = 0; i < orderWrappers.size(); i++) { @@ -228,7 +242,7 @@ public static List loadDifferentInstanceListSorted(Class clazz) { } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadDifferentInstanceListSorted failed", t); + RecordLog.warn("[SpiLoader] ERROR: loadPrototypeInstanceListSorted failed", t); t.printStackTrace(); return new ArrayList<>(); } diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java index ba353fa2d2..0680e2a68d 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java @@ -44,9 +44,17 @@ public void testLoadFirstInstance() { ProcessorSlot processorSlot = SpiLoader.loadFirstInstance(ProcessorSlot.class); assertNotNull(processorSlot); + ProcessorSlot processorSlot2 = SpiLoader.loadFirstInstance(ProcessorSlot.class); + // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances + assertSame(processorSlot, processorSlot2); + SlotChainBuilder slotChainBuilder = SpiLoader.loadFirstInstance(SlotChainBuilder.class); assertNotNull(slotChainBuilder); assertTrue(slotChainBuilder instanceof DefaultSlotChainBuilder); + + SlotChainBuilder slotChainBuilder2 = SpiLoader.loadFirstInstance(SlotChainBuilder.class); + // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances + assertSame(slotChainBuilder, slotChainBuilder2); } @Test @@ -56,6 +64,10 @@ public void testLoadHighestPriorityInstance() { // NodeSelectorSlot is highest order with @SpiOrder(-9000), among all slots assertTrue(processorSlot instanceof NodeSelectorSlot); + + ProcessorSlot processorSlot2 = SpiLoader.loadHighestPriorityInstance(ProcessorSlot.class); + // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances + assertSame(processorSlot, processorSlot2); } @Test @@ -66,14 +78,15 @@ public void testLoadInstanceList() { // Total 8 default slot in sentinel-core assertEquals(8, slots.size()); - // Store the first slot of slots + // Get the first slot of slots ProcessorSlot firstSlot = slots.get(0); // Call loadInstanceList again List slots2 = SpiLoader.loadInstanceList(ProcessorSlot.class); + // Note: the return list are different, and the item instances in list are same assertNotSame(slots, slots2); - // Store the first slot of slots + // Get the first slot of slots2 ProcessorSlot firstSlot2 = slots2.get(0); // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances @@ -100,6 +113,7 @@ public void testLoadInstanceListSorted() { assertTrue(sortedSlots.get(index++) instanceof DegradeSlot); // Verify each call return different instances + // Note: the return list are different, and the item instances in list are same List sortedSlots2 = SpiLoader.loadInstanceListSorted(ProcessorSlot.class); assertNotSame(sortedSlots, sortedSlots2); assertEquals(sortedSlots.size(), sortedSlots2.size()); @@ -107,11 +121,14 @@ public void testLoadInstanceListSorted() { ProcessorSlot slot = sortedSlots.get(i); ProcessorSlot slot2 = sortedSlots2.get(i); assertEquals(slot.getClass(), slot2.getClass()); + + // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances + assertSame(slot, slot2); } } @Test - public void testLoadDifferentInstanceListSorted() { + public void testLoadPrototypeInstanceListSorted() { List sortedSlots = SpiLoader.loadInstanceListSorted(ProcessorSlot.class); assertNotNull(sortedSlots); @@ -129,8 +146,8 @@ public void testLoadDifferentInstanceListSorted() { assertTrue(sortedSlots.get(index++) instanceof FlowSlot); assertTrue(sortedSlots.get(index++) instanceof DegradeSlot); - // Verify each call return different instances - List sortedSlots2 = SpiLoader.loadDifferentInstanceListSorted(ProcessorSlot.class); + // Verify each call return new instances + List sortedSlots2 = SpiLoader.loadPrototypeInstanceListSorted(ProcessorSlot.class); assertNotSame(sortedSlots, sortedSlots2); assertEquals(sortedSlots.size(), sortedSlots2.size()); for (int i = 0; i < sortedSlots.size(); i++) { From 84de476fdb5c7908ec398e11eeddc6aac6ec3781 Mon Sep 17 00:00:00 2001 From: cdfive Date: Sat, 28 Mar 2020 00:25:32 +0800 Subject: [PATCH 2/5] Fix comment --- .../src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java index fe7996ac20..f3569e3e88 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java @@ -167,7 +167,7 @@ public static List loadInstanceList(Class clazz) { } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadInstanceListSorted failed", t); + RecordLog.warn("[SpiLoader] ERROR: loadInstanceList failed", t); t.printStackTrace(); return new ArrayList<>(); } From 2c2375141cebc07458eeca196acf91dd9b491175 Mon Sep 17 00:00:00 2001 From: cdfive Date: Sun, 29 Mar 2020 10:28:49 +0800 Subject: [PATCH 3/5] Use error level in catch block, init ArrayList with capacity and improve add item to list --- .../alibaba/csp/sentinel/util/SpiLoader.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java index f3569e3e88..47b3342a5c 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/SpiLoader.java @@ -60,7 +60,7 @@ public static T loadFirstInstance(Class clazz) { return null; } } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadFirstInstance failed", t); + RecordLog.error("[SpiLoader] ERROR: loadFirstInstance failed", t); t.printStackTrace(); return null; } @@ -95,7 +95,7 @@ public static T loadFirstInstanceOrDefault(Class clazz, Class T loadHighestPriorityInstance(Class clazz) { SpiOrderWrapper w = null; for (T spi : serviceLoader) { int order = SpiOrderResolver.resolveOrder(spi); - RecordLog.info("[SpiLoader] Found {} SPI: {} with order " + order, clazz.getSimpleName(), - spi.getClass().getCanonicalName()); + RecordLog.info("[SpiLoader] Found {} SPI: {} with order {}", clazz.getSimpleName(), + spi.getClass().getCanonicalName(), order); if (w == null || order < w.order) { w = new SpiOrderWrapper<>(order, spi); } } return w == null ? null : w.spi; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadHighestPriorityInstance failed", t); + RecordLog.error("[SpiLoader] ERROR: loadHighestPriorityInstance failed", t); t.printStackTrace(); return null; } @@ -167,7 +167,7 @@ public static List loadInstanceList(Class clazz) { } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadInstanceList failed", t); + RecordLog.error("[SpiLoader] ERROR: loadInstanceList failed", t); t.printStackTrace(); return new ArrayList<>(); } @@ -201,13 +201,13 @@ public static List loadInstanceListSorted(Class clazz) { RecordLog.info("[SpiLoader] Found {} SPI: {} with order {}", clazz.getSimpleName(), spi.getClass().getCanonicalName(), order); } - List list = new ArrayList<>(); + List list = new ArrayList<>(orderWrappers.size()); for (int i = 0; i < orderWrappers.size(); i++) { - list.add(i, orderWrappers.get(i).spi); + list.add(orderWrappers.get(i).spi); } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadInstanceListSorted failed", t); + RecordLog.error("[SpiLoader] ERROR: loadInstanceListSorted failed", t); t.printStackTrace(); return new ArrayList<>(); } @@ -236,13 +236,13 @@ public static List loadPrototypeInstanceListSorted(Class clazz) { RecordLog.debug("[SpiLoader] Found {} SPI: {} with order {}", clazz.getSimpleName(), spi.getClass().getCanonicalName(), order); } - List list = new ArrayList<>(); + List list = new ArrayList<>(orderWrappers.size()); for (int i = 0; i < orderWrappers.size(); i++) { - list.add(i, orderWrappers.get(i).spi); + list.add(orderWrappers.get(i).spi); } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadPrototypeInstanceListSorted failed", t); + RecordLog.error("[SpiLoader] ERROR: loadPrototypeInstanceListSorted failed", t); t.printStackTrace(); return new ArrayList<>(); } From 0b87c4840571323a99934481383f9aeba89b921f Mon Sep 17 00:00:00 2001 From: cdfive Date: Thu, 2 Apr 2020 20:54:56 +0800 Subject: [PATCH 4/5] Remove static block for print log in DefaultSlotChainBuilder --- .../sentinel/slots/DefaultSlotChainBuilder.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java index 33e04c8401..cf0f72c5e5 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java @@ -21,7 +21,6 @@ import com.alibaba.csp.sentinel.slotchain.ProcessorSlot; import com.alibaba.csp.sentinel.slotchain.ProcessorSlotChain; import com.alibaba.csp.sentinel.slotchain.SlotChainBuilder; -import com.alibaba.csp.sentinel.spi.SpiOrder; import com.alibaba.csp.sentinel.util.SpiLoader; import java.util.List; @@ -34,21 +33,6 @@ */ public class DefaultSlotChainBuilder implements SlotChainBuilder { - static { - List sortedSlotList = SpiLoader.loadPrototypeInstanceListSorted(ProcessorSlot.class); - for (ProcessorSlot slot : sortedSlotList) { - String slotClassCanonicalName = slot.getClass().getCanonicalName(); - int order; - if (slot.getClass().isAnnotationPresent(SpiOrder.class)) { - SpiOrder spiOrder = slot.getClass().getAnnotation(SpiOrder.class); - order = spiOrder.value(); - } else { - order = SpiOrder.LOWEST_PRECEDENCE; - } - RecordLog.info("[DefaultSlotChainBuilder]Found ProcessorSlot {} with order {}", slotClassCanonicalName, order); - } - } - @Override public ProcessorSlotChain build() { ProcessorSlotChain chain = new DefaultProcessorSlotChain(); From f8c5bf02118c7e0bc77f20e70f33afa10ff8d392 Mon Sep 17 00:00:00 2001 From: cdfive Date: Thu, 2 Apr 2020 20:57:24 +0800 Subject: [PATCH 5/5] Fix comment in test case --- .../test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java index 0680e2a68d..622f7fd249 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/SpiLoaderTest.java @@ -62,7 +62,7 @@ public void testLoadHighestPriorityInstance() { ProcessorSlot processorSlot = SpiLoader.loadHighestPriorityInstance(ProcessorSlot.class); assertNotNull(processorSlot); - // NodeSelectorSlot is highest order with @SpiOrder(-9000), among all slots + // NodeSelectorSlot is highest order with @SpiOrder(-10000), among all slots assertTrue(processorSlot instanceof NodeSelectorSlot); ProcessorSlot processorSlot2 = SpiLoader.loadHighestPriorityInstance(ProcessorSlot.class);