SNOW-1483230 Parameter support & disable blob encryption for new table format #801

sfc-gh-alhuang · 2024-07-26T21:56:54Z

Split parameter maxChunksInBlobAndRegistrationRequest into maxChunksInBlobs and maxChunksInRegistrationRequest
Merging change of #763 and #773 into main branch.

src/main/java/net/snowflake/ingest/streaming/internal/BlobBuilder.java

src/main/java/net/snowflake/ingest/utils/ParameterProvider.java

src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java

src/test/java/net/snowflake/ingest/streaming/internal/ParameterProviderTest.java

src/test/java/net/snowflake/ingest/streaming/internal/InsertRowsBenchmarkTest.java

src/test/java/net/snowflake/ingest/streaming/internal/OAuthBasicTest.java

src/test/java/net/snowflake/ingest/streaming/internal/BlobBuilderTest.java

sfc-gh-hmadan · 2024-07-31T02:36:24Z

src/main/java/net/snowflake/ingest/utils/ParameterProvider.java

@@ -31,8 +37,9 @@ public class ParameterProvider {
  public static final String MAX_CHUNK_SIZE_IN_BYTES = "MAX_CHUNK_SIZE_IN_BYTES".toLowerCase();
  public static final String MAX_ALLOWED_ROW_SIZE_IN_BYTES =
      "MAX_ALLOWED_ROW_SIZE_IN_BYTES".toLowerCase();
-  public static final String MAX_CHUNKS_IN_BLOB_AND_REGISTRATION_REQUEST =
-      "MAX_CHUNKS_IN_BLOB_AND_REGISTRATION_REQUEST".toLowerCase();
+  public static final String MAX_CHUNKS_IN_BLOB = "MAX_CHUNKS_IN_BLOB".toLowerCase();


How do we know no customer was overriding the old setting ? Any changes to these settings is by default a back compat breaking change that needs to be handled with extra care.
Note that these settings are also public, see here: https://javadoc.io/doc/net.snowflake/snowflake-ingest-sdk/latest/net/snowflake/ingest/utils/ParameterProvider.html

cc @sfc-gh-tzhang

We don't know, I don't think they need to be back compat (at least for this one) since we're not documenting them, and I'm not aware of anyone who is overriding this.

Ok. Lets remove the public access modifier from settings that we do not want customers to be exposed to?
Alec could you please run this by Xin before committing it in?
Do we have telemetry to figure out if any client in the past 6 months has had non-default values set for these parameters?

Lets remove the public access modifier from settings that we do not want customers to be exposed to?

I think we still want to keep it as publicly accessible, I believe it was added for some incident mitigation, but these overrides are never meant to be used long term. + @sfc-gh-lsembera to confirm

Do we have telemetry to figure out if any client in the past 6 months has had non-default values set for these parameters?

I don't think we collect this at server side unfortunately.

This parameter was introduced to fix timeouts when customers ingest into thousands of tables concurrently.

We cannot know if anybody changed the parameter locally, but I am not aware of recommending to change this parameter to any customer.

ok, then I think it's fine to not support backward compatible as these are not publicly documented parameters, customer should not use parameters without our recommendation.

src/main/java/net/snowflake/ingest/utils/ParameterProvider.java

src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java

sfc-gh-tzhang · 2024-08-09T07:00:37Z

src/main/java/net/snowflake/ingest/utils/ParameterProvider.java

@@ -31,8 +37,9 @@ public class ParameterProvider {
  public static final String MAX_CHUNK_SIZE_IN_BYTES = "MAX_CHUNK_SIZE_IN_BYTES".toLowerCase();
  public static final String MAX_ALLOWED_ROW_SIZE_IN_BYTES =
      "MAX_ALLOWED_ROW_SIZE_IN_BYTES".toLowerCase();
-  public static final String MAX_CHUNKS_IN_BLOB_AND_REGISTRATION_REQUEST =
-      "MAX_CHUNKS_IN_BLOB_AND_REGISTRATION_REQUEST".toLowerCase();
+  public static final String MAX_CHUNKS_IN_BLOB = "MAX_CHUNKS_IN_BLOB".toLowerCase();


ok, then I think it's fine to not support backward compatible as these are not publicly documented parameters, customer should not use parameters without our recommendation.

sfc-gh-tzhang

LGTM, thanks!

parameter set & disable blob encryption

a37433d

sfc-gh-alhuang changed the title ~~Parameter support & disable blob encryption for new table format~~ SNOW-1483230 Parameter support & disable blob encryption for new table format Jul 26, 2024

sfc-gh-alhuang marked this pull request as ready for review July 26, 2024 22:04

sfc-gh-alhuang requested review from sfc-gh-tzhang and a team as code owners July 26, 2024 22:04

sfc-gh-alhuang requested a review from sfc-gh-hmadan July 26, 2024 22:04