Skip to content

Commit fb0e300

Browse files
knQzxknQzx
authored
add --init-period and --init-max-failures flags (#980)
* add init-period flag for faster initial sync retries * address review: alphabetize flags, add readme docs, add e2e tests * move init flags to alphabetical order in man text * fix init-timeout e2e test to actually trigger init timeout * switch init-timeout to init-max-failures per review - rename flag and helper to count-based (int, not duration) - skip --max-failures during init when init-max-failures is set so main-max does not override init tolerance - add SYNC PHASES section to --man and README describing init vs steady-state - update e2e test to use a bad server path, as suggested in review * address review: simplify closure, use pflag.Changed for init-max-failures - move flInitMaxFailures next to flMaxFailures - default flInitPeriod to flPeriod to kill conditional in hot path - replace isInitFailuresExceeded + useMainLimit with getMaxFailures closure - track waitTime as a loop-outer variable, flip on init-done - detect --init-max-failures via pflag.Changed || GITSYNC_INIT_MAX_FAILURES env; -1 = unlimited, 0 = abort immediately, unset = fall through to --max-failures - drop now-stale unit tests for removed helpers, update --man and README
1 parent d86d141 commit fb0e300

3 files changed

Lines changed: 151 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,24 @@ CONTRACT
206206
use as little disk space as possible (see the --depth and --git-gc flags),
207207
but this is not part of the contract.
208208
209+
SYNC PHASES
210+
211+
git-sync operates in two phases:
212+
213+
Initial sync:
214+
git-sync retries until its first successful sync with the remote
215+
repo. During this phase, the retry interval is controlled by
216+
--init-period (falling back to --period if unset) and the failure
217+
limit is controlled by --init-max-failures (falling back to
218+
--max-failures when unset). This phase is useful for tolerating
219+
transient connectivity issues at startup while still giving up
220+
eventually.
221+
222+
Steady state:
223+
Once the first sync succeeds, git-sync polls the remote at the
224+
--period interval and tolerates failures up to --max-failures before
225+
aborting. --init-period and --init-max-failures no longer apply.
226+
209227
OPTIONS
210228
211229
Many options can be specified as either a commandline flag or an environment
@@ -365,6 +383,20 @@ OPTIONS
365383
Enable the pprof debug endpoints on git-sync's HTTP endpoint at
366384
/debug/pprof. Requires --http-bind to be specified.
367385
386+
--init-max-failures <int>, $GITSYNC_INIT_MAX_FAILURES
387+
The number of consecutive failures allowed before aborting during
388+
the initial sync phase (before the first successful sync). Once
389+
the initial sync succeeds, --max-failures applies instead.
390+
Setting this to a negative value will retry forever during the
391+
initial sync. If this flag is not set, --max-failures applies
392+
to the initial sync phase as well.
393+
394+
--init-period <duration>, $GITSYNC_INIT_PERIOD
395+
How long to wait between sync attempts until the first successful
396+
sync. Once the initial sync succeeds, --period is used instead.
397+
This must be at least 10ms if set. If not specified, --period is
398+
used for all sync attempts.
399+
368400
--link <string>, $GITSYNC_LINK
369401
The path to at which to create a symlink which points to the
370402
current git directory, at the currently synced hash. This may be

‎main.go‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,9 @@ func main() {
183183
flErrorFile := pflag.String("error-file",
184184
envString("", "GITSYNC_ERROR_FILE", "GIT_SYNC_ERROR_FILE"),
185185
"the path (absolute or relative to --root) to an optional file into which errors will be written (defaults to disabled)")
186+
flInitPeriod := pflag.Duration("init-period",
187+
envDuration(0, "GITSYNC_INIT_PERIOD"),
188+
"how long to wait between sync attempts until the first success, must be >= 10ms if set; if unset, --period is used")
186189
flPeriod := pflag.Duration("period",
187190
envDuration(10*time.Second, "GITSYNC_PERIOD", "GIT_SYNC_PERIOD"),
188191
"how long to wait between syncs, must be >= 10ms; --wait overrides this")
@@ -198,6 +201,9 @@ func main() {
198201
flMaxFailures := pflag.Int("max-failures",
199202
envInt(0, "GITSYNC_MAX_FAILURES", "GIT_SYNC_MAX_FAILURES"),
200203
"the number of consecutive failures allowed before aborting (-1 will retry forever")
204+
flInitMaxFailures := pflag.Int("init-max-failures",
205+
envInt(0, "GITSYNC_INIT_MAX_FAILURES"),
206+
"the number of consecutive failures allowed before aborting during the initial sync phase; a negative value retries forever; if unset, --max-failures applies instead")
201207
flTouchFile := pflag.String("touch-file",
202208
envString("", "GITSYNC_TOUCH_FILE", "GIT_SYNC_TOUCH_FILE"),
203209
"the path (absolute or relative to --root) to an optional file which will be touched whenever a sync completes (defaults to disabled)")
@@ -376,6 +382,11 @@ func main() {
376382

377383
pflag.Parse()
378384

385+
// Was --init-max-failures explicitly set (CLI or env)? envInt applies
386+
// the env value as the pflag default, so pflag.Changed alone misses it.
387+
_, initMaxFailuresEnv := os.LookupEnv("GITSYNC_INIT_MAX_FAILURES")
388+
initMaxFailuresSet := initMaxFailuresEnv || pflag.Lookup("init-max-failures").Changed
389+
379390
// Handle print-and-exit cases.
380391
if *flVersion {
381392
fmt.Fprintln(os.Stdout, version.VERSION)
@@ -469,6 +480,11 @@ func main() {
469480
if *flPeriod < 10*time.Millisecond {
470481
fatalConfigErrorf(log, true, "invalid flag: --period must be at least 10ms")
471482
}
483+
if *flInitPeriod == 0 {
484+
*flInitPeriod = *flPeriod
485+
} else if *flInitPeriod < 10*time.Millisecond {
486+
fatalConfigErrorf(log, true, "invalid flag: --init-period must be at least 10ms")
487+
}
472488

473489
if *flDeprecatedChmod != 0 {
474490
fatalConfigErrorf(log, true, "deprecated flag: --change-permissions is no longer supported")
@@ -912,20 +928,37 @@ func main() {
912928

913929
failCount := 0
914930
syncCount := uint64(0)
931+
initialSyncDone := false
932+
waitTime := *flInitPeriod
933+
// getMaxFailures returns the effective max-failure limit for the current
934+
// phase. During the initial sync phase, --init-max-failures (if set)
935+
// overrides --max-failures; otherwise --max-failures applies.
936+
getMaxFailures := func() int {
937+
if !initialSyncDone && initMaxFailuresSet {
938+
return *flInitMaxFailures
939+
}
940+
return *flMaxFailures
941+
}
915942
for {
916943
start := time.Now()
917944
ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout)
918945

919946
if changed, hash, err := git.SyncRepo(ctx, refreshCreds); err != nil {
920947
failCount++
921948
updateSyncMetrics(metricKeyError, start)
922-
if *flMaxFailures >= 0 && failCount >= *flMaxFailures {
923-
// Exit after too many retries, maybe the error is not recoverable.
924-
log.Error(err, "too many failures, aborting", "failCount", failCount)
949+
if maxFails := getMaxFailures(); maxFails >= 0 && failCount >= maxFails {
950+
log.Error(err, "too many failures, aborting", "failCount", failCount, "maxFailures", maxFails)
925951
os.Exit(1)
926952
}
927953
log.Error(err, "error syncing repo, will retry", "failCount", failCount)
928954
} else {
955+
if !initialSyncDone {
956+
initialSyncDone = true
957+
waitTime = *flPeriod
958+
if *flInitPeriod != *flPeriod {
959+
log.V(0).Info("initial sync complete, switching to normal period", "initPeriod", flInitPeriod.String(), "period", flPeriod.String())
960+
}
961+
}
929962
// this might have been called before, but also might not have
930963
setRepoReady()
931964
// We treat the first loop as a sync, including sending hooks.
@@ -989,12 +1022,12 @@ func main() {
9891022
log.DeleteErrorFile()
9901023
}
9911024

992-
log.V(3).Info("next sync", "waitTime", flPeriod.String(), "syncCount", syncCount)
1025+
log.V(3).Info("next sync", "waitTime", waitTime.String(), "syncCount", syncCount)
9931026
cancel()
9941027

9951028
// Sleep until the next sync. If syncSig is set then the sleep may
9961029
// be interrupted by that signal.
997-
t := time.NewTimer(*flPeriod)
1030+
t := time.NewTimer(waitTime)
9981031
select {
9991032
case <-t.C:
10001033
case <-sigChan:
@@ -2397,6 +2430,24 @@ CONTRACT
23972430
use as little disk space as possible (see the --depth and --git-gc flags),
23982431
but this is not part of the contract.
23992432
2433+
SYNC PHASES
2434+
2435+
git-sync operates in two phases:
2436+
2437+
Initial sync:
2438+
git-sync retries until its first successful sync with the remote
2439+
repo. During this phase, the retry interval is controlled by
2440+
--init-period (falling back to --period if unset) and the failure
2441+
limit is controlled by --init-max-failures (falling back to
2442+
--max-failures when unset). This phase is useful for tolerating
2443+
transient connectivity issues at startup while still giving up
2444+
eventually.
2445+
2446+
Steady state:
2447+
Once the first sync succeeds, git-sync polls the remote at the
2448+
--period interval and tolerates failures up to --max-failures before
2449+
aborting. --init-period and --init-max-failures no longer apply.
2450+
24002451
OPTIONS
24012452
24022453
Many options can be specified as either a commandline flag or an environment
@@ -2556,6 +2607,20 @@ OPTIONS
25562607
Enable the pprof debug endpoints on git-sync's HTTP endpoint at
25572608
/debug/pprof. Requires --http-bind to be specified.
25582609
2610+
--init-max-failures <int>, $GITSYNC_INIT_MAX_FAILURES
2611+
The number of consecutive failures allowed before aborting during
2612+
the initial sync phase (before the first successful sync). Once
2613+
the initial sync succeeds, --max-failures applies instead.
2614+
Setting this to a negative value will retry forever during the
2615+
initial sync. If this flag is not set, --max-failures applies
2616+
to the initial sync phase as well.
2617+
2618+
--init-period <duration>, $GITSYNC_INIT_PERIOD
2619+
How long to wait between sync attempts until the first successful
2620+
sync. Once the initial sync succeeds, --period is used instead.
2621+
This must be at least 10ms if set. If not specified, --period is
2622+
used for all sync attempts.
2623+
25592624
--link <string>, $GITSYNC_LINK
25602625
The path to at which to create a symlink which points to the
25612626
current git directory, at the currently synced hash. This may be

‎test_e2e.sh‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3691,6 +3691,55 @@ function e2e::exechook_git_archive() {
36913691
assert_tgz_archive "$ROOT/link/archive.tgz"
36923692
}
36933693

3694+
##############################################
3695+
# Test init-period uses faster interval for initial sync
3696+
##############################################
3697+
function e2e::init_period_faster_initial_sync() {
3698+
# First sync
3699+
echo "${FUNCNAME[0]} 1" > "$REPO/file"
3700+
git -C "$REPO" commit -qam "${FUNCNAME[0]} 1"
3701+
3702+
GIT_SYNC \
3703+
--period=100s \
3704+
--init-period=100ms \
3705+
--repo="file://$REPO" \
3706+
--root="$ROOT" \
3707+
--link="link" \
3708+
&
3709+
# With init-period=100ms, sync should happen quickly even though
3710+
# period=100s. If init-period were not working, this would time out.
3711+
wait_for_sync "${MAXWAIT}"
3712+
assert_link_exists "$ROOT/link"
3713+
assert_file_exists "$ROOT/link/file"
3714+
assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]} 1"
3715+
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 1
3716+
3717+
# After initial sync, period should switch to the normal 100s.
3718+
# Make a new commit and verify it does NOT sync quickly (because
3719+
# we're now using the slow period).
3720+
echo "${FUNCNAME[0]} 2" > "$REPO/file"
3721+
git -C "$REPO" commit -qam "${FUNCNAME[0]} 2"
3722+
# Wait a bit - should NOT have synced since normal period is 100s
3723+
sleep 3
3724+
assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]} 1"
3725+
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 1
3726+
}
3727+
3728+
##############################################
3729+
# Test init-max-failures aborts after N failed attempts
3730+
##############################################
3731+
function e2e::init_max_failures_exceeded() {
3732+
assert_fail \
3733+
GIT_SYNC \
3734+
--period=100ms \
3735+
--init-max-failures=3 \
3736+
--max-failures=-1 \
3737+
--repo="file:///does/not/exist" \
3738+
--root="$ROOT" \
3739+
--link="link"
3740+
assert_file_absent "$ROOT/link/file"
3741+
}
3742+
36943743
#
36953744
# main
36963745
#

0 commit comments

Comments
 (0)