Added TotalRows and TotalBytes to the /status api for Ghostferry
#303
Conversation
EtienneBerubeShopify
requested review from
shuhaowu,
paarthmadan,
Manan007224 and
floriecai
and removed request for
paarthmadan
StateTracker is used only for critical state that is needed to reconstruct Ghostferry after it is interrupted and resumed. Things that can be discovered should not be included in that struct. See the |
EtienneBerubeShopify
force-pushed
the
etienne-add-total-bytes-and-total-rows
branch
3 times, most recently
from
d01fc2f
to
be58a07
Compare
EtienneBerubeShopify
force-pushed
the
etienne-add-total-bytes-and-total-rows
branch
from
3d990fa
to
38b631c
Compare
| @@ -116,7 +116,7 @@ func (c *DatabaseConfig) MySQLConfig() (*mysql.Config, error) { | |||
| return nil, fmt.Errorf("failed to build TLS config: %v", err) | |||
| } | |||
|
|
|||
| cfgName := fmt.Sprintf("%s@%s:%s", c.User, c.Host, c.Port) | |||
| cfgName := fmt.Sprintf("%s@%s:%d", c.User, c.Host, c.Port) | |||
There was a problem hiding this comment.
is this change made purposefully ?
There was a problem hiding this comment.
Yes, the compiler was complaining and wouldnt compile until this was fixed
| @@ -337,7 +337,7 @@ func (v *IterativeVerifier) reverifyUntilStoreIsSmallEnough(maxIterations int) e | |||
| before := v.reverifyStore.RowCount | |||
| start := time.Now() | |||
|
|
|||
| _, err := v.verifyStore("reverification_before_cutover", []MetricTag{{"iteration", string(iteration)}}) | |||
| _, err := v.verifyStore("reverification_before_cutover", []MetricTag{{"iteration", fmt.Sprint(iteration)}}) | |||
There was a problem hiding this comment.
Also is this change made purposefully ?
There was a problem hiding this comment.
Yes, the compiler was complaining and wouldnt compile until this was fixed
string(int) creates a rune with the ascii / UTF-8 int code
| var totalBytes sqlorig.NullInt64 | ||
|
|
||
| for rows.Next() { | ||
| if err = rows.Scan(&totalRows, &totalBytes); err != nil { |
There was a problem hiding this comment.
Is there a reason why you're not returning the error from here and making the stat values as 0 ?
There was a problem hiding this comment.
I thought about what should happen when either a connection fails or there is an error with the row scanning, and in all cases, I don't want to stop the execution to report an error for one row. Even a level above, the only way of dealing with this would be to set it to zero regardless. The result could also be omitted, and the value in the map would still be zero.
There was a problem hiding this comment.
Errors happened while scanning would have nothing to do with issues in connection. Although I get your reasoning on why you're not returning from the error, but just for the sake of consistency with codebase we should return the errors.
There was a problem hiding this comment.
Update: Paired with Manan
IMO fetching the stats via the |
This was an artifact from the first way of doing it, wanted to keep it for the documentation, but I guess it just brings more confusion. |
| totalRowsPerTable, totalBytesPerTable, err := f.GetTotalRowsAndBytesMap() | ||
|
|
||
| if err != nil { | ||
| f.logger.Error(err) |
There was a problem hiding this comment.
If this error occurs, won't line 996 below experience a nil pointer error?
There was a problem hiding this comment.
It shouldn't as the maps are already initialized in GetTotalRowsAndBytesMap(). And uninitialized keys return zero values (for uint64 it returns 0). So if the error occurred you'll be getting zeroes for the TotalRows and TotalBytes
Added TotalRows and TotalBytes to the
/statusapi for Ghostferry for better time estimations when logging with Splunk.This included getting the data from the
information_schemaand adding the data into theStateTrackerso that it could be serialized later to be sent via HTTP.DEBATE (DEPRECATED):
Should it fetch the data even when running from an interrupted state (reading from a serialized state) or should it only query the data when coming from a "clean" run?Notes:
The Go compiler was complaining about a few things and would not allow me to run any commands before it was happy so I had to fix a few warnings. Here are the files with these "fixes":