Copilot commented on code in PR #13345: URL: https://github.com/apache/cloudstack/pull/13345#discussion_r4457630901
########## utils/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java: ########## @@ -0,0 +1,120 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.threadcontext; + +import com.cloud.utils.StringUtils; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.ThreadContext; + +import java.lang.reflect.Type; +import java.util.HashMap; +import java.util.Map; + +/** + * Utility class, helps to propagate {@link ThreadContext} values from parent to child threads. + * + * @author mprokopchuk + */ +public class ThreadContextUtil { + private static final Logger logger = LogManager.getLogger(ThreadContextUtil.class); + + public static final String MDC_UUID_KEY = "uuid"; + public static final String MDC_LOG_CONTEXT_ID_KEY = "logcontextid"; + public static final String CONTEXT_UUID_KEY = "uuid"; + public static final String CONTEXT_LOG_ID_KEY = "logid"; + + /** + * Wrap {@link Runnable} to propagate {@link ThreadContext} values. + * + * @param delegate the runnable to execute with the calling thread's {@link ThreadContext} values + * @return a runnable that installs the captured context, runs the delegate and restores the previous context + */ + public static Runnable wrapThreadContext(Runnable delegate) { + Map<String, String> context = ThreadContext.getContext() != null ? + new HashMap<>(ThreadContext.getContext()) : null; + + return () -> { + Map<String, String> oldContext = ThreadContext.getContext() != null ? + new HashMap<>(ThreadContext.getContext()) : null; + try { + ThreadContext.clearMap(); + if (context != null) { + context.forEach(ThreadContext::put); + } + delegate.run(); + } finally { + ThreadContext.clearMap(); + if (oldContext != null) { + oldContext.forEach(ThreadContext::put); + } + } + }; + } + + /** + * Set UUID in ThreadContext. + * + * @param uuid the UUID value to set + */ + public static void setUuid(String uuid) { + if (StringUtils.isNotEmpty(uuid)) { + ThreadContext.put(MDC_UUID_KEY, uuid); + } Review Comment: This leaves the previous UUID in Log4j's thread-local context when `uuid` is blank. `ApiDispatcher` calls this for every request, and the API worker threads are reused without a corresponding ThreadContext clear, so a request without a UUID can inherit and propagate the UUID from the preceding request to async jobs and agent commands. Remove the MDC key when the argument is empty, as `ThreadContextCommandUtil.putOrRemove` does. ########## engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql: ########## @@ -651,3 +651,16 @@ WHERE `name`='user.vm.readonly.details' AND `value` IS NOT NULL; -- usage records introduced in 4.22.1 (cumulative and per-VM) can coexist. See #13399. CALL `cloud_usage`.`IDEMPOTENT_DROP_INDEX`('id', 'cloud_usage.usage_volume'); CALL `cloud_usage`.`IDEMPOTENT_ADD_UNIQUE_INDEX`('cloud_usage.usage_volume', 'id', '(volume_id ASC, created ASC, vm_id ASC)'); + +-- these indexes created from top 20 resource consuming DB queries +CALL `cloud`.`IDEMPOTENT_ADD_INDEX`('idx_mshost_removed_state','cloud.mshost', '(removed, state)'); +CALL `cloud`.`IDEMPOTENT_ADD_INDEX`('idx_host_id_removed','cloud.host', '(id, removed)'); +CALL `cloud`.`IDEMPOTENT_ADD_INDEX`('idx_networks_id_removed','cloud.networks', '(id, removed)'); +CALL `cloud`.`IDEMPOTENT_ADD_INDEX`('idx_alert_type_data_center_id_archived_pod_id','cloud.alert', '(type, data_center_id, archived, pod_id)'); +CALL `cloud`.`IDEMPOTENT_ADD_INDEX`('idx_configuration_name','cloud.configuration', '(name)'); Review Comment: `cloud.configuration` already has the `i_configuration__name` index added by `schema-228to229.sql:62`. Adding another single-column index on the same column under a new name provides no query benefit but increases storage and write cost for every configuration change. Please remove this duplicate index (or replace the existing one rather than adding a second copy). -- 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]
