From e1435974c6c592128401778f98b2b3d80ccbb89a Mon Sep 17 00:00:00 2001 From: cdfive <31885791+cdfive@users.noreply.github.com> Date: Fri, 3 Apr 2020 15:46:25 +0800 Subject: [PATCH] Set default log level of JDK logging to INFO and polish code of SpiLoader (#1365) * Improve log info in SpiLoader, improve comment and test case * Use error level in catch block, init ArrayList with capacity and improve add item to list --- .../csp/sentinel/log/jul/BaseJulLogger.java | 4 +- .../slots/DefaultSlotChainBuilder.java | 2 +- .../alibaba/csp/sentinel/util/SpiLoader.java | 54 ++++++++++++------- .../csp/sentinel/util/SpiLoaderTest.java | 29 +++++++--- 4 files changed, 61 insertions(+), 28 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 e8a1fe6f..0490efd2 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 @@ public class BaseJulLogger { 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 797206bb..cf0f72c5 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 @@ -38,7 +38,7 @@ public class DefaultSlotChainBuilder implements SlotChainBuilder { 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 a677084e..47b3342a 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 { @@ -52,7 +60,7 @@ public final class SpiLoader { return null; } } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadFirstInstance failed", t); + RecordLog.error("[SpiLoader] ERROR: loadFirstInstance failed", t); t.printStackTrace(); return null; } @@ -87,7 +95,7 @@ public final class SpiLoader { } return defaultClass.newInstance(); } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadFirstInstanceOrDefault failed", t); + RecordLog.error("[SpiLoader] ERROR: loadFirstInstanceOrDefault failed", t); t.printStackTrace(); return null; } @@ -96,6 +104,8 @@ public final class SpiLoader { /** * Load the SPI instance with highest priority. * + * Note: each call return same instances. + * * @param clazz class of the SPI * @param SPI type * @return the SPI instance with highest priority if exists, or else false @@ -114,15 +124,15 @@ public final class SpiLoader { 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; } @@ -132,6 +142,8 @@ public final class SpiLoader { * 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,11 +161,13 @@ public final class SpiLoader { List list = new ArrayList<>(); for (T spi : serviceLoader) { + RecordLog.info("[SpiLoader] Found {} SPI: {}", clazz.getSimpleName(), + spi.getClass().getCanonicalName()); list.add(spi); } return list; } catch (Throwable t) { - RecordLog.warn("[SpiLoader] ERROR: loadInstanceListSorted failed", t); + RecordLog.error("[SpiLoader] ERROR: loadInstanceList failed", t); t.printStackTrace(); return new ArrayList<>(); } @@ -162,7 +176,7 @@ public final class SpiLoader { /** * 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,32 +198,32 @@ public final class SpiLoader { 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<>(); + 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<>(); } } /** - * 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,16 +233,16 @@ public final class SpiLoader { 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<>(); + 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: loadDifferentInstanceListSorted failed", t); + RecordLog.error("[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 ba353fa2..622f7fd2 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 class SpiLoaderTest { 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 @@ -54,8 +62,12 @@ public class SpiLoaderTest { 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); + // As SERVICE_LOADER_MAP in SpiLoader cached the instance, so they're same instances + assertSame(processorSlot, processorSlot2); } @Test @@ -66,14 +78,15 @@ public class SpiLoaderTest { // 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 class SpiLoaderTest { 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 class SpiLoaderTest { 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 class SpiLoaderTest { 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++) {