improve a bit more

src/backend/distributed/shared_library_init.c

@@ -2903,13 +2903,13 @@ ApplicationNameAssignHook(const char *newval, void *extra)
 	 * the global pid, we cannot do so if we might need to read from catalog
 	 * but it's unsafe to do so. For Citus internal backends, this cannot be the
 	 * case because in that case AssignGlobalPID() just extracts its global pid
-	 * from the application_name. But for external backends, we either need to
-	 * guarantee that we won't read from catalog tables or that it's safe to do
-	 * so. The only case where AssignGlobalPID() could read from catalog tables
-	 * is when the cached local node id is invalidated. So for this reason, for
-	 * external client backends, we require that either the cached local node id
-	 * is valid or that we are in a transaction block -because in that case it's
-	 * safe to read from catalog.
+	 * from the application_name and extracting doesn't require catalog access.
+	 * But for external client backends, we either need to guarantee that we won't
+	 * read from catalog tables or that it's safe to do so. The only case where
+	 * AssignGlobalPID() could read from catalog tables is when the cached local
+	 * node id is invalidated. So for this reason, for external client backends,
+	 * we require that either the cached local node id is valid or that we are in
+	 * a transaction block -because in that case it's safe to read from catalog.
 	 *
 	 * Another solution to the catalog table acccess problem would be to update
 	 * global pid lazily, like we do for HideShards. But that's not possible

src/test/regress/expected/remove_coordinator.out

@@ -20,16 +20,15 @@ ERROR:  connection to the remote node postgres@200.200.200.200:1 failed
   -- transaction block while Postgres does that.
   --
   -- And when the application_name changes, Citus tries to re-assign
-  -- the global pid but it does so only for Citus internal backends,
-  -- and doing so for Citus internal backends doesn't require being
-  -- in a transaction block and is safe.
+  -- the global pid and doing so for Citus internal backends doesn't
+  -- require being in a transaction block and is safe.
   --
   -- However, for the client external backends (like us here), Citus
-  -- doesn't re-assign the global pid because it's not safe to do so
-  -- outside of a transaction block. This is because, it would require
-  -- performing a catalog access to retrive the local node id when the
-  -- cached local node is invalidated like what just happened here
-  -- because of the failed citus_add_node() call made above.
+  -- doesn't try to re-assign the global pid if doing so requires catalog
+  -- access and we're outside of a transaction block. Note that in that
+  -- case the catalog access may only be needed to retrive the local node
+  -- id when the cached local node is invalidated like what just happened
+  -- here because of the failed citus_add_node() call made above.
   --
   -- So by failing here (rather than crashing), we ensure this behavior.
 ROLLBACK;

src/test/regress/sql/remove_coordinator.sql

@@ -19,16 +19,15 @@ BEGIN;
   -- transaction block while Postgres does that.
   --
   -- And when the application_name changes, Citus tries to re-assign
-  -- the global pid but it does so only for Citus internal backends,
-  -- and doing so for Citus internal backends doesn't require being
-  -- in a transaction block and is safe.
+  -- the global pid and doing so for Citus internal backends doesn't
+  -- require being in a transaction block and is safe.
   --
   -- However, for the client external backends (like us here), Citus
-  -- doesn't re-assign the global pid because it's not safe to do so
-  -- outside of a transaction block. This is because, it would require
-  -- performing a catalog access to retrive the local node id when the
-  -- cached local node is invalidated like what just happened here
-  -- because of the failed citus_add_node() call made above.
+  -- doesn't try to re-assign the global pid if doing so requires catalog
+  -- access and we're outside of a transaction block. Note that in that
+  -- case the catalog access may only be needed to retrive the local node
+  -- id when the cached local node is invalidated like what just happened
+  -- here because of the failed citus_add_node() call made above.
   --
   -- So by failing here (rather than crashing), we ensure this behavior.
 ROLLBACK;