You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/03/01 17:02:22 UTC

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6569: T3C Race Condition Update

ocket8888 commented on a change in pull request #6569:
URL: https://github.com/apache/trafficcontrol/pull/6569#discussion_r816961701



##########
File path: docs/source/api/v2/servers_hostname_update_status.rst
##########
@@ -52,16 +52,26 @@ Response Structure
 ------------------
 Each object in the returned array\ [1]_ will contain the following fields:
 
+:configUpdateTime:     The last time an update was requested for this server. This field defaults to standard epoch
+:configApplyTime:      The last time an update was applied for this server. This field defaults to standard epoch
 :host_id:              The integral, unique identifier for the server for which the other fields in this object represent the pending updates and revalidation status
 :host_name:            The (short) hostname of the server for which the other fields in this object represent the pending updates and revalidation status
 :parent_pending:       A boolean telling whether or not the :term:`parents` of this server have pending updates
 :parent_reval_pending: A boolean telling whether or not the :term:`parents` of this server have pending :term:`Content Invalidation Jobs`
 :reval_pending:        ``true`` if the server has pending :term:`Content Invalidation Jobs`, ``false`` otherwise
-:status:               The name of the status of this server
+
+    .. note:: While not officially deprecated, this is based on the values corresponding to ``revalUpdateTime`` and ``revalApplyTime``. It is preferred to rely on the timestamp fields going forward as this will likely be deprecated in the future.

Review comment:
       We should just deprecate this right now in APIv3

##########
File path: docs/source/api/v2/servers_hostname_update_status.rst
##########
@@ -52,16 +52,26 @@ Response Structure
 ------------------
 Each object in the returned array\ [1]_ will contain the following fields:
 
+:configUpdateTime:     The last time an update was requested for this server. This field defaults to standard epoch
+:configApplyTime:      The last time an update was applied for this server. This field defaults to standard epoch

Review comment:
       New fields cannot be added to released API versions.

##########
File path: lib/go-tc/cachegroups.go
##########
@@ -91,7 +87,6 @@ type CachegroupTrimmedName struct {
 // CachegroupQueueUpdatesRequest holds info relating to the
 // cachegroups/{{ID}}/queue_update TO route.
 type CachegroupQueueUpdatesRequest struct {
-	Action string           `json:"action"`
-	CDN    *CDNName         `json:"cdn"`
-	CDNID  *util.JSONIntStr `json:"cdnId"`
+	Action string `json:"action"`
+	CDNID  *int   `json:"cdnId"`

Review comment:
       This is a breaking API client change that will affect released API versions

##########
File path: traffic_ops/app/db/migrations/2022021523082100_server_config_update.up.sql
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+CREATE TABLE IF NOT EXISTS public.server_config_update ( 

Review comment:
       There's no need for one-to-many relationships - in fact that should be prohibited - so why not make these just columns on the existing `public.server` table?

##########
File path: traffic_ops/app/db/migrations/2022021523082100_server_config_update.up.sql
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+CREATE TABLE IF NOT EXISTS public.server_config_update ( 
+    server_id bigint NOT NULL PRIMARY KEY, 
+    config_update_time TIMESTAMPTZ, 
+    config_apply_time TIMESTAMPTZ, 
+    revalidate_update_time TIMESTAMPTZ, 
+    revalidate_apply_time TIMESTAMPTZ, 
+    last_updated TIMESTAMPTZ NOT NULL DEFAULT now(),
+    CONSTRAINT fk_server_id FOREIGN KEY(server_id) REFERENCES public.server(id) ON DELETE CASCADE
+);
+
+ALTER TABLE public.server DROP COLUMN IF EXISTS upd_pending, DROP COLUMN IF EXISTS reval_pending;

Review comment:
       Missing newline at EOF




-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org