From 8097685436fd3df473dd7f6721a572742c5f1cd4 Mon Sep 17 00:00:00 2001 From: tangenta Date: Tue, 28 Jun 2022 16:06:39 +0800 Subject: [PATCH 1/2] helper: request another PD if one of them is unavailable (#35750) close pingcap/tidb#35708 --- store/helper/helper.go | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/store/helper/helper.go b/store/helper/helper.go index e3912ac9aff26..948a84c84635d 100644 --- a/store/helper/helper.go +++ b/store/helper/helper.go @@ -840,42 +840,46 @@ func (h *Helper) requestPD(apiName, method, uri string, body io.Reader, res inte if len(pdHosts) == 0 { return errors.New("pd unavailable") } - logutil.BgLogger().Debug("RequestPD URL", zap.String("url", util.InternalHTTPSchema()+"://"+pdHosts[0]+uri)) - req := new(http.Request) for _, host := range pdHosts { - req, err = http.NewRequest(method, util.InternalHTTPSchema()+"://"+host+uri, body) - if err != nil { - // Try to request from another PD node when some nodes may down. - if strings.Contains(err.Error(), "connection refused") { - continue - } - return errors.Trace(err) + err = requestPDForOneHost(host, apiName, method, uri, body, res) + if err == nil { + break } + // Try to request from another PD node when some nodes may down. } + return err +} + +func requestPDForOneHost(host, apiName, method, uri string, body io.Reader, res interface{}) error { + urlVar := fmt.Sprintf("%s://%s%s", util.InternalHTTPSchema(), host, uri) + logutil.BgLogger().Debug("RequestPD URL", zap.String("url", urlVar)) + req, err := http.NewRequest(method, urlVar, body) if err != nil { - return err + logutil.BgLogger().Warn("requestPDForOneHost new request failed", + zap.String("url", urlVar), zap.Error(err)) + return errors.Trace(err) } start := time.Now() resp, err := util.InternalHTTPClient().Do(req) if err != nil { metrics.PDAPIRequestCounter.WithLabelValues(apiName, "network error").Inc() + logutil.BgLogger().Warn("requestPDForOneHost do request failed", + zap.String("url", urlVar), zap.Error(err)) return errors.Trace(err) } metrics.PDAPIExecutionHistogram.WithLabelValues(apiName).Observe(time.Since(start).Seconds()) metrics.PDAPIRequestCounter.WithLabelValues(apiName, resp.Status).Inc() - defer func() { err = resp.Body.Close() if err != nil { - logutil.BgLogger().Error("close body failed", zap.Error(err)) + logutil.BgLogger().Warn("requestPDForOneHost close body failed", + zap.String("url", urlVar), zap.Error(err)) } }() - err = json.NewDecoder(resp.Body).Decode(res) if err != nil { return errors.Trace(err) } - return nil } From 27e7bbdd4e04d01869426a023ef1efaf85497c7e Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Tue, 28 Jun 2022 02:40:40 -0600 Subject: [PATCH 2/2] sessionctx/variable: add tests to ensure skipInit can be removed (#35703) ref pingcap/tidb#35051 --- sessionctx/variable/sysvar_test.go | 86 ++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/sessionctx/variable/sysvar_test.go b/sessionctx/variable/sysvar_test.go index fb821c9f5cf78..68d68e42b159c 100644 --- a/sessionctx/variable/sysvar_test.go +++ b/sessionctx/variable/sysvar_test.go @@ -674,6 +674,92 @@ func TestSettersandGetters(t *testing.T) { } } +// TestSkipInitIsUsed ensures that no new variables are added with skipInit: true. +// This feature is deprecated, and if you need to run code to differentiate between init and "SET" (rare), +// you can instead check if s.StmtCtx.StmtType == "Set". +// The reason it is deprecated is that the behavior is typically wrong: +// it means session settings won't inherit from global and don't apply until you first set +// them in each session. This is a very weird behavior. +// See: https://github.com/pingcap/tidb/issues/35051 +func TestSkipInitIsUsed(t *testing.T) { + for _, sv := range GetSysVars() { + if sv.skipInit { + // Many of these variables might allow skipInit to be removed, + // they need to be checked first. The purpose of this test is to make + // sure we don't introduce any new variables with skipInit, which seems + // to be a problem. + switch sv.Name { + case Timestamp, + WarningCount, + ErrorCount, + LastInsertID, + Identity, + TiDBTxnScope, + TiDBSnapshot, + TiDBOptDistinctAggPushDown, + TiDBOptWriteRowID, + TiDBChecksumTableConcurrency, + TiDBBatchInsert, + TiDBBatchDelete, + TiDBBatchCommit, + TiDBCurrentTS, + TiDBLastTxnInfo, + TiDBLastQueryInfo, + TiDBEnableChunkRPC, + TxnIsolationOneShot, + TiDBOptimizerSelectivityLevel, + TiDBOptimizerEnableOuterJoinReorder, + TiDBLogFileMaxDays, + TiDBConfig, + TiDBDDLReorgPriority, + TiDBSlowQueryFile, + TiDBWaitSplitRegionFinish, + TiDBWaitSplitRegionTimeout, + TiDBLowResolutionTSO, + TiDBAllowRemoveAutoInc, + TiDBMetricSchemaStep, + TiDBMetricSchemaRangeDuration, + TiDBFoundInPlanCache, + TiDBFoundInBinding, + RandSeed1, + RandSeed2, + TiDBLastDDLInfo, + TiDBGeneralLog, + TiDBSlowLogThreshold, + TiDBRecordPlanInSlowLog, + TiDBEnableSlowLog, + TiDBCheckMb4ValueInUTF8, + TiDBPProfSQLCPU, + TiDBDDLSlowOprThreshold, + TiDBForcePriority, + TiDBMemoryUsageAlarmRatio, + TiDBEnableCollectExecutionInfo, + TiDBPersistAnalyzeOptions, + TiDBEnableColumnTracking, + TiDBStatsLoadPseudoTimeout, + SQLLogBin, + ForeignKeyChecks, + CollationDatabase, + CharacterSetClient, + CharacterSetResults, + CollationConnection, + CharsetDatabase, + GroupConcatMaxLen, + CharacterSetConnection, + CharacterSetServer, + TiDBBuildStatsConcurrency, + TiDBOptTiFlashConcurrencyFactor, + TiDBOptSeekFactor, + TiDBOptJoinReorderThreshold, + TiDBStatsLoadSyncWait, + CharacterSetFilesystem: + continue + } + require.Equal(t, false, sv.skipInit, fmt.Sprintf("skipInit should not be set on new system variables. variable %s is in violation", sv.Name)) + } + } +} + func TestSecureAuth(t *testing.T) { sv := GetSysVar(SecureAuth) vars := NewSessionVars()