Collect aggregates for fbid/hashout tables

[tinaLuofb]

Collect aggregates for fbid/hashout tables during mysqldump. This change is similar to 8200e5e?diff=unified

[tinaLuofb]

Hi @santoshbanda, can you help me review this change? Thanks!

[facebook-github-bot]

@santoshbanda has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[mdcallag]

Should this be visible to the public?
What happens when columns change their offsets in rows (see
stat_field_offset)?

On Tue, Nov 1, 2016 at 2:44 PM, Facebook Community Bot <
[email protected]> wrote:

@santoshbanda https://github.com/santoshbanda has imported this pull
request. If you are a Facebook employee, you can view this diff on
Phabricator https://phabricator.intern.facebook.com/D4113658.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#383 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABkKTeA933ZsolR9OezD3H5552CcBWwWks5q57KmgaJpZM4Kml6N
.

Mark Callaghan
[email protected]

[mdcallag]

Also, how do you guard against referencing beyond the array when you assume
the array has at least this many fields for tables that match the hardcoded
names?
FBID_IS_DELETED_FIELD 4

On Tue, Nov 1, 2016 at 3:01 PM, MARK CALLAGHAN [email protected] wrote:

Should this be visible to the public?
What happens when columns change their offsets in rows (see
stat_field_offset)?

On Tue, Nov 1, 2016 at 2:44 PM, Facebook Community Bot <
[email protected]> wrote:

@santoshbanda https://github.com/santoshbanda has imported this pull
request. If you are a Facebook employee, you can view this diff on
Phabricator https://phabricator.intern.facebook.com/D4113658.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#383 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABkKTeA933ZsolR9OezD3H5552CcBWwWks5q57KmgaJpZM4Kml6N
.

Mark Callaghan
[email protected]

Mark Callaghan
[email protected]

[tinaLuofb]

Hi @mdcallag , Thanks for reviewing this change! Yeah, if the column offsets get changed, the stats will be incorrect. but I think this schema change will rarely happen, if so, we can detect that from the stats, since the data must be quite different.

[tinaLuofb]

yeah. That's true. It is not a good idea to do hardcoding. Since the table name is unique, only FBID table will get matched, and I think deleting column of FBID table will rarely happen, if so, mysqldump will throw some error, and then we can fix that. If you have other better suggestions, I am happy to take it. Thanks!

[mdcallag]

I don't see where the code confirms that the offsets used to reference
row[] are valid. For example, how do you confirm this isn't beyond the end
of the array?
row[FBID_IS_DELETED_FIELD]

On Tue, Nov 1, 2016 at 3:21 PM, tinaLuofb [email protected] wrote:

yeah. That's true. It is not a good idea to do hardcoding. Since the table
name is unique, only FBID table will get matched, and I think deleting
column of FBID table will rarely happen, if so, mysqldump will throw some
error, and then we can fix that. If you have other better suggestions, I am
happy to take it. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#383 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABkKTaV04ff4KAGJj5d9PDbF_X6H6H5uks5q57tqgaJpZM4Kml6N
.

Mark Callaghan
[email protected]

[santoshbanda]

I don't think it is a good idea to throw errors in mysqldump if some columns are removed or modified. You should handle these edge cases and ignore collecting the stats and let the mysqldump succeed.

[facebook-github-bot]

@tinaLuofb updated the pull request - view changes - changes since last import

[tinaLuofb]

Hi @mdcallag @santoshbanda I just updated my change to handle the edge case, let me know if there is other suggestions. Thanks for reviewing this!

[facebook-github-bot]

@tinaLuofb updated the pull request - view changes - changes since last import

[facebook-github-bot]

@tinaLuofb updated the pull request - view changes - changes since last import

[facebook-github-bot]

@tinaLuofb updated the pull request - view changes - changes since last import