Copilot commented on code in PR #8163:
URL: https://github.com/apache/incubator-seata/pull/8163#discussion_r3575665353
##########
common/src/main/java/org/apache/seata/common/util/HttpClientUtil.java:
##########
@@ -66,50 +65,52 @@ public class HttpClientUtil {
private static final int HTTP2_WATCH_READ_TIMEOUT_SECONDS_DEFAULT = 300;
static {
- Runtime.getRuntime().addShutdownHook(new Thread(() -> {
- HTTP_CLIENT_MAP.values().parallelStream().forEach(client -> {
- try {
- // Delay 3 seconds to ensure unregister HTTP requests are
sent successfully
- Thread.sleep(3000);
- client.dispatcher().executorService().shutdown();
- // Wait for up to 3 seconds for in-flight requests to
complete
- if
(!client.dispatcher().executorService().awaitTermination(3, TimeUnit.SECONDS)) {
- LOGGER.warn("Timeout waiting for OkHttp executor
service to terminate.");
- }
- client.connectionPool().evictAll();
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- LOGGER.error("Interrupted while waiting for OkHttp
executor service to terminate.", e);
- } catch (Exception e) {
- LOGGER.error(e.getMessage(), e);
- }
- });
-
- HTTP2_CLIENT_MAP.values().parallelStream().forEach(client -> {
- try {
- client.dispatcher().executorService().shutdown();
- // Wait for up to 3 seconds for in-flight requests to
complete
- if
(!client.dispatcher().executorService().awaitTermination(3, TimeUnit.SECONDS)) {
- LOGGER.warn("Timeout waiting for OkHttp executor
service to terminate.");
- }
- client.connectionPool().evictAll();
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- LOGGER.error("Interrupted while waiting for OkHttp
executor service to terminate.", e);
- } catch (Exception e) {
- LOGGER.error(e.getMessage(), e);
- }
- });
- }));
+ Runtime.getRuntime()
+ .addShutdownHook(new
NamedThreadFactory("http-client-shutdown", 1, false)
+ .newThread(HttpClientUtil::shutdownHttpClients));
+ }
+
+ private static void shutdownHttpClients() {
+ for (OkHttpClient client : HTTP_CLIENT_MAP.values()) {
+ shutdownHttpClient(client, true);
+ }
+ for (OkHttpClient client : HTTP2_CLIENT_MAP.values()) {
+ shutdownHttpClient(client, false);
+ }
+ }
Review Comment:
shutdownHttpClients() now sleeps 3 seconds per OkHttpClient in
HTTP_CLIENT_MAP (because shutdownHttpClient(..., true) is called in a loop). If
multiple clients exist (e.g., per-timeout instances), JVM shutdown time becomes
N*3s instead of a single ~3s delay, which is a measurable shutdown regression
compared to the previous parallel shutdown behavior.
##########
json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonUtil.java:
##########
@@ -84,9 +123,56 @@ public static <T> T parseObject(String text, Class<T>
clazz) {
if (Objects.isNull(text) || Objects.isNull(clazz)) {
return null;
}
- String jsonParseName =
text.startsWith(Constants.JACKSON_JSON_TEXT_PREFIX)
+ return getJsonSerializer(text).parseObject(text, clazz);
+ }
+
+ /**
+ * Deserialize the given JSON string to an object of the specified class.
+ *
+ * @param <T> the type of the object
+ * @param text the JSON string
+ * @param clazz the class to deserialize to
+ * @param ignoreAutoType whether to ignore auto type information
+ * @return the deserialized object
+ * @throws JsonParseException if deserialization fails
+ */
+ public static <T> T parseObject(String text, Class<T> clazz, boolean
ignoreAutoType) {
+ if (Objects.isNull(text) || Objects.isNull(clazz)) {
+ return null;
+ }
+ return getJsonSerializer(text).parseObject(text, clazz,
ignoreAutoType);
+ }
+
+ /**
+ * Deserialize the given JSON string to an object of the specified type.
+ *
+ * @param <T> the type of the object
+ * @param text the JSON string
+ * @param type the type to deserialize to
+ * @return the deserialized object
+ * @throws JsonParseException if deserialization fails
+ */
+ public static <T> T parseObjectWithType(String text, Type type) {
+ if (Objects.isNull(text) || Objects.isNull(type)) {
+ return null;
+ }
+ return getJsonSerializer(text).parseObjectWithType(text, type);
+ }
+
+ /**
+ * Check whether the given JSON string uses auto type information.
+ *
+ * @param json the JSON string to check
+ * @return true if auto type is used, otherwise false
+ */
+ public static boolean useAutoType(String json) {
+ return DEFAULT_SERIALIZER.useAutoType(json);
+ }
Review Comment:
JsonUtil.useAutoType() currently delegates to DEFAULT_SERIALIZER only. This
ignores the legacy Jackson marker (Constants.JACKSON_JSON_TEXT_PREFIX) that
JsonUtil.getJsonSerializer() uses to route parsing, so JSON strings starting
with the Jackson prefix can be mis-detected as not using auto-type (e.g., when
the configured serializer's useAutoType() only checks for "@type"). This can
break the "compatible history autoType" flows that rely on useAutoType() to
decide ignoreAutoType.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
