Add PostgreSQL system identifier and YugabyteDB cluster UUID to callhome #2990
Conversation
ShivanshGahlot
changed the title
ShivanshGahlot
requested review from
sanyamsinghal,
makalaaneesh and
priyanshi-yb
and removed request for
sanyamsinghal and
makalaaneesh
yb-voyager/src/srcdb/source.go
Outdated
| if s.DBType == "postgresql" { | ||
| if pgDB, ok := s.DB().(*PostgreSQL); ok { | ||
| if systemIdentifier, err := pgDB.GetSystemIdentifier(); err == nil { | ||
| s.PostgresSystemIdentifier = systemIdentifier | ||
| } else { | ||
| log.Infof("callhome: failed to get PostgreSQL system identifier: %v", err) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
You can use the Query / QueryRow function of srcdb.go to run the system query directly in source commands.
There was a problem hiding this comment.
Done. Remove the function and instead doing that directly here.
| yb.EnsureConnected() | ||
| yb.Lock() | ||
| defer yb.Unlock() |
There was a problem hiding this comment.
Why do we need to use locks? I think we are fetching this information only once at the start, right?
There was a problem hiding this comment.
We were using the same pattern in GetVersion so I just followed that. If you feel I should remove the locks here then I can do that here.
There was a problem hiding this comment.
Okay, yeah, I think EnsureConnected is okay, lock is not required here we can remove it from GetVersion also. But lets skip for now
yb-voyager/src/tgtdb/yugabytedb.go
Outdated
| if strings.Contains(err.Error(), "column") && strings.Contains(err.Error(), "does not exist") { | ||
| log.Infof("YugabyteDB cluster UUID not available (version < v2024.2.3.0)") |
There was a problem hiding this comment.
what is the error msg here?
There was a problem hiding this comment.
I have added it as a comment too:
ERROR: column "universe_uuid" does not exist
There was a problem hiding this comment.
I think we can check the complete error message, right?
ShivanshGahlot
force-pushed
the
shivansh/database-identifier
branch
from
b0d8c8c to
cd5d600
Compare
| DBType: source.DBType, | ||
| DBVersion: source.DBVersion, | ||
| DBSize: source.DBSize, | ||
| PostgresSystemIdentifier: source.PostgresSystemIdentifier, |
There was a problem hiding this comment.
I would recommend not using "Postgres" in the key/field name. This should be source-agnostic, and re-usable in the future.
There was a problem hiding this comment.
Hmm I was confused about this. But yeah I'll make it source agnostic
yb-voyager/src/srcdb/source.go
Outdated
| // FetchDBSystemIdentifier fetches and stores the database system identifier | ||
| // Currently only implemented for PostgreSQL | ||
| func (s *Source) FetchDBSystemIdentifier() { | ||
| if s.DBType == "postgresql" { |
| yb.EnsureConnected() | ||
| yb.Lock() | ||
| defer yb.Unlock() |
There was a problem hiding this comment.
Okay, yeah, I think EnsureConnected is okay, lock is not required here we can remove it from GetVersion also. But lets skip for now
yb-voyager/src/tgtdb/yugabytedb.go
Outdated
| if strings.Contains(err.Error(), "column") && strings.Contains(err.Error(), "does not exist") { | ||
| log.Infof("YugabyteDB cluster UUID not available (version < v2024.2.3.0)") |
There was a problem hiding this comment.
I think we can check the complete error message, right?
Describe the changes in this pull request
This PR enhances the call home by adding two new diagnostic fields:
Callhome data contains
db_system_identifierfield which contains these values.These additions provide better identification and tracking capabilities for migration operations.
Describe if there are any user-facing changes
None
How was this pull request tested?
Manually
Does your PR have changes in callhome/yugabyted payloads? If so, is the payload version incremented?
Yes the two new fields that have been added.
"source_db_details":"{\"host\":\"\",\"db_type\":\"postgresql\",\"db_version\":\"17.2 (Ubuntu 17.2-1.pgdg22.04+1)\",\"total_db_size_bytes\":466944,\"db_system_identifier\":7449834921171374629}""target_db_details":"{\"host\":\"\",\"db_version\":\"15.12-YB-2025.1.0.1-b0\",\"node_count\":1,\"total_cores\":8,\"db_system_identifier\":\"d74f0c96-2474-4e63-b290-bdc29527a3d9\"}"Does your PR have changes that can cause upgrade issues?