From 63513b44f2268215271608140255336c96594aab Mon Sep 17 00:00:00 2001 From: haoyann <43994656+haoyann@users.noreply.github.com> Date: Fri, 15 Jan 2021 19:26:47 +0800 Subject: [PATCH] [ISSUE #4661]configServletInner#doGetConfig code optimization (#4666) * configServletInner#doGetConfig code optimization * add comment * update comment * update comment --- .../server/controller/ConfigServletInner.java | 59 +++++++++---------- .../server/service/ConfigCacheService.java | 6 +- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java b/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java index b7cc2da2e6f..85c490b778c 100755 --- a/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java +++ b/config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java @@ -128,25 +128,23 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon final String requestIp = RequestUtil.getRemoteIp(request); boolean isBeta = false; if (lockResult > 0) { + // LockResult > 0 means cacheItem is not null and other thread can`t delete this cacheItem FileInputStream fis = null; try { String md5 = Constants.NULL; long lastModified = 0L; CacheItem cacheItem = ConfigCacheService.getContentCache(groupKey); - if (cacheItem != null) { - if (cacheItem.isBeta()) { - if (cacheItem.getIps4Beta().contains(clientIp)) { - isBeta = true; - } - } - - final String configType = - (null != cacheItem.getType()) ? cacheItem.getType() : FileTypeEnum.TEXT.getFileType(); - response.setHeader("Config-Type", configType); - FileTypeEnum fileTypeEnum = FileTypeEnum.getFileTypeEnumByFileExtensionOrFileType(configType); - String contentTypeHeader = fileTypeEnum.getContentType(); - response.setHeader(HttpHeaderConsts.CONTENT_TYPE, contentTypeHeader); + if (cacheItem.isBeta() && cacheItem.getIps4Beta().contains(clientIp)) { + isBeta = true; } + + final String configType = + (null != cacheItem.getType()) ? cacheItem.getType() : FileTypeEnum.TEXT.getFileType(); + response.setHeader("Config-Type", configType); + FileTypeEnum fileTypeEnum = FileTypeEnum.getFileTypeEnumByFileExtensionOrFileType(configType); + String contentTypeHeader = fileTypeEnum.getContentType(); + response.setHeader(HttpHeaderConsts.CONTENT_TYPE, contentTypeHeader); + File file = null; ConfigInfoBase configInfoBase = null; PrintWriter out = null; @@ -162,13 +160,11 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon } else { if (StringUtils.isBlank(tag)) { if (isUseTag(cacheItem, autoTag)) { - if (cacheItem != null) { - if (cacheItem.tagMd5 != null) { - md5 = cacheItem.tagMd5.get(autoTag); - } - if (cacheItem.tagLastModifiedTs != null) { - lastModified = cacheItem.tagLastModifiedTs.get(autoTag); - } + if (cacheItem.tagMd5 != null) { + md5 = cacheItem.tagMd5.get(autoTag); + } + if (cacheItem.tagLastModifiedTs != null) { + lastModified = cacheItem.tagLastModifiedTs.get(autoTag); } if (PropertyUtil.isDirectRead()) { configInfoBase = persistService.findConfigInfo4Tag(dataId, group, tenant, autoTag); @@ -202,15 +198,13 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon } } } else { - if (cacheItem != null) { - if (cacheItem.tagMd5 != null) { - md5 = cacheItem.tagMd5.get(tag); - } - if (cacheItem.tagLastModifiedTs != null) { - Long lm = cacheItem.tagLastModifiedTs.get(tag); - if (lm != null) { - lastModified = lm; - } + if (cacheItem.tagMd5 != null) { + md5 = cacheItem.tagMd5.get(tag); + } + if (cacheItem.tagLastModifiedTs != null) { + Long lm = cacheItem.tagLastModifiedTs.get(tag); + if (lm != null) { + lastModified = lm; } } if (PropertyUtil.isDirectRead()) { @@ -301,7 +295,12 @@ public String doGetConfig(HttpServletRequest request, HttpServletResponse respon private static void releaseConfigReadLock(String groupKey) { ConfigCacheService.releaseReadLock(groupKey); } - + + /** + * Try to add read lock. + * @param groupKey groupKey string value. + * @return 0 - No data and failed. Positive number - lock succeeded. Negative number - lock failed。 + */ private static int tryConfigReadLock(String groupKey) { // Lock failed by default. diff --git a/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java b/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java index 0a525b28566..6cc420cf5e5 100644 --- a/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java +++ b/config/src/main/java/com/alibaba/nacos/config/server/service/ConfigCacheService.java @@ -607,11 +607,11 @@ public static boolean isUptodate(String groupKey, String md5, String ip, String } /** - * Try to add read lock. If it successed, then it can call {@link #releaseWriteLock(String)}.And it won't call if + * Try to add read lock. If it succeeded, then it can call {@link #releaseWriteLock(String)}.And it won't call if * failed. * * @param groupKey groupKey string value. - * @return 0 - No data and failed. Positive number 0 - Success. Negative number - lock failed。 + * @return 0 - No data and failed. Positive number - lock succeeded. Negative number - lock failed。 */ public static int tryReadLock(String groupKey) { CacheItem groupItem = CACHE.get(groupKey); @@ -635,7 +635,7 @@ public static void releaseReadLock(String groupKey) { } /** - * Try to add write lock. If it successed, then it can call {@link #releaseWriteLock(String)}.And it won't call if + * Try to add write lock. If it succeeded, then it can call {@link #releaseWriteLock(String)}.And it won't call if * failed. * * @param groupKey groupKey string value.