fix(mysql): enforce non-zero initial wait time for CDC setup

[Kamlesh72]

Description

Fail the setup if initial_wait_time is 0 or negative

Fixes #492

Screenshots or Recordings

UI

CLI

[CLAassistant]

All committers have signed the CLA.

[vaibhav-datazip]

similar check needs to be added in Postgres as well

[Kamlesh72]

@vaibhav-datazip Currently in postgres if User provide initial_wait_time to 0, we set it to default value ( 1200 ).

Instead of that, If User provide initial_wait_time we validate it [ 120, 2400 ].
If User doesn't provide initial_wait_time then we set to 1200.

Does that works?

olake/drivers/postgres/internal/postgres.go

Lines 101 to 114 in 1001235

	found, _ := utils.IsOfType(p.config.UpdateMethod, "replication_slot")
	if found {
	logger.Info("Found CDC Configuration")
	cdc := &CDC{}
	if err := utils.Unmarshal(p.config.UpdateMethod, cdc); err != nil {
	return err
	}
	// set default value
	cdc.InitialWaitTime = utils.Ternary(cdc.InitialWaitTime == 0, 1200, cdc.InitialWaitTime).(int)
	// check if initial wait time is valid or not
	if cdc.InitialWaitTime < 120 || cdc.InitialWaitTime > 2400 {
	return fmt.Errorf("the CDC initial wait time must be at least 120 seconds and less than 2400 seconds")
	}

[vaibhav-datazip]

@Kamlesh72 , one more check needs to be added here , if cdc config is selected and Olake mysql cdc setup permissions are missing or not granted, then too the check should fail when clicked next

[Kamlesh72]

@vaibhav-datazip Shouldn't we check cdcSupported only if user has provided initial_wait_time and fail setup if cdc is not supported ?

https://github.com/datazip-inc/olake/blob/a79f88af07fb97e3b9181d906585b7965e9172cf/drivers/mysql/internal/mysql.go#L121-130

[vaibhav-datazip]

@vaibhav-datazip Shouldn't we check cdcSupported only if user has provided initial_wait_time and fail setup if cdc is not supported ?

https://github.com/datazip-inc/olake/blob/a79f88af07fb97e3b9181d906585b7965e9172cf/drivers/mysql/internal/mysql.go#L121-130

exactly, if user has selected cdc mode instead of standalone mode (speaking in terms of UI), we should add a check that cdc conditions are met. If these condition checks fail then we should throw error, like binlog permissions alongwith correct cdc initial wait time.

[Kamlesh72]

@vaibhav-datazip Added mysql permission check and postgres checks.

[vaibhav-datazip]

why Is this change done ??

[Kamlesh72]

olake/drivers/mysql/internal/mysql.go

Lines 135 to 139 in 93a6da4

	// Enable CDC support if binlog is configured
	cdcSupported, err := m.IsCDCSupported(ctx)
	if err != nil {
	return err
	}

So if error occurs, user can see error like binlog_format is not set to ROW

[vaibhav-datazip]

any reason for this change ?

[Kamlesh72]

If user leave Initial wait time field empty, we set to 1200.
If user enters any value ( including 0 ), then only we validate.

So if user enters 0, I thought we shouldn't silently change that to default value.

[vaibhav-datazip]

also why no query based check is added in postgres side.

you can visit the olake documentation on how to verify cdc configurations

SELECT name, setting FROM pg_settings 
WHERE name IN ('wal_level', 'max_replication_slots', 'max_wal_senders');

[Kamlesh72]

My mistake I didnt checked that. I will include it in code.

[Kamlesh72]

Will update this by tomorrow ( saturday ).

[vaibhav-datazip]

any updates on this @Kamlesh72 ?

[vaibhav-datazip]

hi @Kamlesh72 , will be closing this PR if you are no longer working on this

[Kamlesh72]

@vaibhav-datazip I will surely merge it this week. Sorry for the delay.

[vaibhav-datazip]

@vaibhav-datazip I will surely merge it this week. Sorry for the delay.

okay then, no problem . please resolve the merge conflicts and acknowledge the previous comments