Browse Source

The code has been got rid of the TS3ReadActorFactoryConfig (#8181)

Oleg Doronin 6 months ago
parent
commit
f30d4812e0

+ 2 - 7
ydb/core/fq/libs/actors/run_actor.cpp

@@ -779,11 +779,7 @@ private:
             mkqlDefaultLimit = 8_GB;
         }
 
-        // This part is for backward compatibility. TODO: remove this part after migration to TS3GatewayConfig
-        auto s3ReadDefaultInflightLimit = Params.Config.GetReadActorsFactoryConfig().GetS3ReadActorFactoryConfig().GetDataInflight();
-        if (s3ReadDefaultInflightLimit == 0) {
-            s3ReadDefaultInflightLimit = Params.Config.GetGateways().GetS3().GetDataInflight();
-        }
+        auto s3ReadDefaultInflightLimit = Params.Config.GetGateways().GetS3().GetDataInflight();
         if (s3ReadDefaultInflightLimit == 0) {
             s3ReadDefaultInflightLimit = 200_MB;
         }
@@ -1970,8 +1966,7 @@ private:
         }
 
         {
-           dataProvidersInit.push_back(GetS3DataProviderInitializer(Params.S3Gateway, Params.CredentialsFactory,
-                Params.Config.GetReadActorsFactoryConfig().HasS3ReadActorFactoryConfig() ? Params.Config.GetReadActorsFactoryConfig().GetS3ReadActorFactoryConfig().GetAllowLocalFiles() : Params.Config.GetGateways().GetS3().GetAllowLocalFiles(), NActors::TActivationContext::ActorSystem())); // This part is for backward compatibility. TODO: remove this part after migration to TS3GatewayConfig
+           dataProvidersInit.push_back(GetS3DataProviderInitializer(Params.S3Gateway, Params.CredentialsFactory, NActors::TActivationContext::ActorSystem()));
         }
 
         {

+ 1 - 9
ydb/core/fq/libs/config/protos/read_actors_factory.proto

@@ -8,19 +8,11 @@ import "ydb/library/yql/providers/s3/proto/retry_config.proto";
 
 ////////////////////////////////////////////////////////////
 
-message TS3ReadActorFactoryConfig {
-    NYql.NS3.TRetryConfig RetryConfig = 1;
-    uint64 RowsInBatch = 2; // Default = 1000
-    uint64 MaxInflight = 3; // Default = 20
-    uint64 DataInflight = 4; // Default = 200 MB
-    bool AllowLocalFiles = 5;
-}
-
 message TPqReadActorFactoryConfig {
     bool CookieCommitMode = 1; // Turn off RangesMode setting in PQ read session.
 }
 
 message TReadActorsFactoryConfig {
-    TS3ReadActorFactoryConfig S3ReadActorFactoryConfig = 1;
+    reserved 1;
     TPqReadActorFactoryConfig PqReadActorFactoryConfig = 2;
 }

+ 0 - 12
ydb/core/fq/libs/init/init.cpp

@@ -196,21 +196,9 @@ void Init(
     auto s3ActorsFactory = NYql::NDq::CreateS3ActorsFactory();
 
     if (protoConfig.GetPrivateApi().GetEnabled()) {
-        const auto& s3readConfig = protoConfig.GetReadActorsFactoryConfig().GetS3ReadActorFactoryConfig();
         auto s3HttpRetryPolicy = NYql::GetHTTPDefaultRetryPolicy(NYql::THttpRetryPolicyOptions{.MaxTime = TDuration::Max(), .RetriedCurlCodes = NYql::FqRetriedCurlCodes()});
         NYql::NDq::TS3ReadActorFactoryConfig readActorFactoryCfg = NYql::NDq::CreateReadActorFactoryConfig(protoConfig.GetGateways().GetS3());
 
-        // These fillings were left for the backward compatibility. TODO: remove this part after migration to TS3GatewayConfig
-        if (const ui64 rowsInBatch = s3readConfig.GetRowsInBatch()) {
-            readActorFactoryCfg.RowsInBatch = rowsInBatch;
-        }
-        if (const ui64 maxInflight = s3readConfig.GetMaxInflight()) {
-            readActorFactoryCfg.MaxInflight = maxInflight;
-        }
-        if (const ui64 dataInflight = s3readConfig.GetDataInflight()) {
-            readActorFactoryCfg.DataInflight = dataInflight;
-        }
-
         RegisterDqInputTransformLookupActorFactory(*asyncIoFactory);
         RegisterDqPqReadActorFactory(*asyncIoFactory, yqSharedResources->UserSpaceYdbDriver, credentialsFactory, yqCounters->GetSubgroup("subsystem", "DqSourceTracker"));
         RegisterYdbReadActorFactory(*asyncIoFactory, yqSharedResources->UserSpaceYdbDriver, credentialsFactory);

+ 2 - 3
ydb/library/yql/providers/s3/provider/yql_s3_provider.cpp

@@ -4,8 +4,8 @@
 
 namespace NYql {
 
-TDataProviderInitializer GetS3DataProviderInitializer(IHTTPGateway::TPtr gateway, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, bool allowLocalFiles, NActors::TActorSystem* actorSystem) {
-    return [gateway, credentialsFactory, allowLocalFiles, actorSystem] (
+TDataProviderInitializer GetS3DataProviderInitializer(IHTTPGateway::TPtr gateway, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, NActors::TActorSystem* actorSystem) {
+    return [gateway, credentialsFactory, actorSystem] (
         const TString& userName,
         const TString& sessionId,
         const TGatewaysConfig* gatewaysConfig,
@@ -35,7 +35,6 @@ TDataProviderInitializer GetS3DataProviderInitializer(IHTTPGateway::TPtr gateway
         if (gatewaysConfig) {
             state->Configuration->Init(gatewaysConfig->GetS3(), typeCtx);
         }
-        state->Configuration->AllowLocalFiles = allowLocalFiles;
         state->Gateway = gateway;
 
         TDataProviderInfo info;

+ 1 - 1
ydb/library/yql/providers/s3/provider/yql_s3_provider.h

@@ -35,7 +35,7 @@ struct TS3State : public TThrRefBase
     NActors::TActorSystem* ActorSystem = nullptr;
 };
 
-TDataProviderInitializer GetS3DataProviderInitializer(IHTTPGateway::TPtr gateway, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory = nullptr, bool allowLocalFiles = false, NActors::TActorSystem* actorSystem = nullptr);
+TDataProviderInitializer GetS3DataProviderInitializer(IHTTPGateway::TPtr gateway, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory = nullptr, NActors::TActorSystem* actorSystem = nullptr);
 
 TIntrusivePtr<IDataProvider> CreateS3DataSource(TS3State::TPtr state);
 TIntrusivePtr<IDataProvider> CreateS3DataSink(TS3State::TPtr state);

+ 2 - 1
ydb/library/yql/tools/dqrun/dqrun.cpp

@@ -956,13 +956,14 @@ int RunMain(int argc, const char* argv[])
     }
 
     if (gatewaysConfig.HasS3()) {
+        gatewaysConfig.MutableS3()->SetAllowLocalFiles(true);
         for (auto& cluster: gatewaysConfig.GetS3().GetClusterMapping()) {
             clusters.emplace(to_lower(cluster.GetName()), TString{S3ProviderName});
         }
         if (!httpGateway) {
             httpGateway = IHTTPGateway::Make(gatewaysConfig.HasHttpGateway() ? &gatewaysConfig.GetHttpGateway() : nullptr);
         }
-        dataProvidersInit.push_back(GetS3DataProviderInitializer(httpGateway, nullptr, true, nullptr));
+        dataProvidersInit.push_back(GetS3DataProviderInitializer(httpGateway, nullptr));
     }
 
     if (gatewaysConfig.HasPq()) {

+ 1 - 1
ydb/tests/fq/common/conftest.py

@@ -18,9 +18,9 @@ from ydb.tests.tools.fq_runner.kikimr_utils import start_kikimr
 @pytest.fixture
 def kikimr(request: pytest.FixtureRequest, yq_version: str):
     kikimr_extensions = [
+        AddFormatSizeLimitExtension(),
         AddInflightExtension(),
         AddDataInflightExtension(),
-        AddFormatSizeLimitExtension(),
         DefaultConfigExtension(''),
         YQv2Extension(yq_version),
         ComputeExtension(),

+ 1 - 1
ydb/tests/fq/plans/conftest.py

@@ -61,9 +61,9 @@ def s3(request) -> S3:
 @pytest.fixture
 def kikimr(request: pytest.FixtureRequest, s3: S3, yq_version: str, stats_mode: str):
     kikimr_extensions = [
+        AddFormatSizeLimitExtension(),
         AddInflightExtension(),
         AddDataInflightExtension(),
-        AddFormatSizeLimitExtension(),
         DefaultConfigExtension(s3.s3_url),
         YQv2Extension(yq_version),
         ComputeExtension(),

+ 1 - 1
ydb/tests/fq/restarts/conftest.py

@@ -66,9 +66,9 @@ def stats_mode():
 @pytest.fixture
 def kikimr(request: pytest.FixtureRequest, s3: S3, yq_version: str, stats_mode: str):
     kikimr_extensions = [
+        AddFormatSizeLimitExtension(),
         AddInflightExtension(),
         AddDataInflightExtension(),
-        AddFormatSizeLimitExtension(),
         DefaultConfigExtension(s3.s3_url),
         YQv2Extension(yq_version),
         ComputeExtension(),

+ 1 - 1
ydb/tests/fq/s3/conftest.py

@@ -87,9 +87,9 @@ def kikimr_params(request: pytest.FixtureRequest):
 
 def get_kikimr_extensions(s3: S3, yq_version: str, kikimr_settings, mvp_external_ydb_endpoint):
     return [
+        AddFormatSizeLimitExtension(),
         AddInflightExtension(),
         AddDataInflightExtension(),
-        AddFormatSizeLimitExtension(),
         DefaultConfigExtension(s3.s3_url),
         YQv2Extension(yq_version, kikimr_settings.get("is_replace_if_exists", False)),
         ComputeExtension(),

Some files were not shown because too many files changed in this diff