diff --git a/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java b/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java index 9456ca2db..869f734eb 100644 --- a/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java +++ b/src/main/java/net/snowflake/ingest/connection/RequestBuilder.java @@ -295,7 +295,8 @@ public RequestBuilder( SecurityManager securityManager, CloseableHttpClient httpClient, String clientName) { - this(accountName, + this( + accountName, userName, credential, schemeName, @@ -648,7 +649,7 @@ private static void addUserAgent(HttpUriRequest request, String userAgentSuffix) public void addToken(HttpUriRequest request) { request.setHeader(HttpHeaders.AUTHORIZATION, BEARER_PARAMETER + securityManager.getToken()); request.setHeader(SF_HEADER_AUTHORIZATION_TOKEN_TYPE, this.securityManager.getTokenType()); - if(addAccountNameInRequest) { + if (addAccountNameInRequest) { request.setHeader(SF_HEADER_ACCOUNT_NAME, accountName); } } diff --git a/src/main/java/net/snowflake/ingest/streaming/DropChannelRequest.java b/src/main/java/net/snowflake/ingest/streaming/DropChannelRequest.java index 4fe476edf..3bc096c16 100644 --- a/src/main/java/net/snowflake/ingest/streaming/DropChannelRequest.java +++ b/src/main/java/net/snowflake/ingest/streaming/DropChannelRequest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 Snowflake Computing Inc. All rights reserved. + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. */ package net.snowflake.ingest.streaming; @@ -20,9 +20,6 @@ public class DropChannelRequest { // Name of the table that the channel belongs to private final String tableName; - // Optional client sequencer to verify when dropping the channel. - private final Long clientSequencer; - public static DropChannelRequestBuilder builder(String channelName) { return new DropChannelRequestBuilder(channelName); } @@ -34,8 +31,6 @@ public static class DropChannelRequestBuilder { private String schemaName; private String tableName; - private Long clientSequencer = null; - public DropChannelRequestBuilder(String channelName) { this.channelName = channelName; } @@ -55,17 +50,12 @@ public DropChannelRequestBuilder setTableName(String tableName) { return this; } - public DropChannelRequestBuilder setClientSequencer(Long clientSequencer) { - this.clientSequencer = clientSequencer; - return this; - } - public DropChannelRequest build() { return new DropChannelRequest(this); } } - private DropChannelRequest(DropChannelRequestBuilder builder) { + public DropChannelRequest(DropChannelRequestBuilder builder) { Utils.assertStringNotNullOrEmpty("channel name", builder.channelName); Utils.assertStringNotNullOrEmpty("database name", builder.dbName); Utils.assertStringNotNullOrEmpty("schema name", builder.schemaName); @@ -75,7 +65,6 @@ private DropChannelRequest(DropChannelRequestBuilder builder) { this.dbName = builder.dbName; this.schemaName = builder.schemaName; this.tableName = builder.tableName; - this.clientSequencer = builder.clientSequencer; } public String getDBName() { @@ -97,8 +86,4 @@ public String getChannelName() { public String getFullyQualifiedTableName() { return String.format("%s.%s.%s", this.dbName, this.schemaName, this.tableName); } - - public Long getClientSequencer() { - return this.clientSequencer; - } } diff --git a/src/main/java/net/snowflake/ingest/streaming/OpenChannelRequest.java b/src/main/java/net/snowflake/ingest/streaming/OpenChannelRequest.java index b103d046c..0cf16af2e 100644 --- a/src/main/java/net/snowflake/ingest/streaming/OpenChannelRequest.java +++ b/src/main/java/net/snowflake/ingest/streaming/OpenChannelRequest.java @@ -43,10 +43,6 @@ public enum OnErrorOption { private final String offsetToken; private final boolean isOffsetTokenProvided; - // If true, the channel will be dropped when it is closed after any pending data is fully - // committed. - private final boolean dropOnClose; - public static OpenChannelRequestBuilder builder(String channelName) { return new OpenChannelRequestBuilder(channelName); } @@ -63,8 +59,6 @@ public static class OpenChannelRequestBuilder { private String offsetToken; private boolean isOffsetTokenProvided = false; - private boolean dropOnClose = false; - public OpenChannelRequestBuilder(String channelName) { this.channelName = channelName; this.defaultTimezone = DEFAULT_DEFAULT_TIMEZONE; @@ -101,11 +95,6 @@ public OpenChannelRequestBuilder setOffsetToken(String offsetToken) { return this; } - public OpenChannelRequestBuilder setDropOnClose(boolean dropOnClose) { - this.dropOnClose = dropOnClose; - return this; - } - public OpenChannelRequest build() { return new OpenChannelRequest(this); } @@ -127,7 +116,6 @@ private OpenChannelRequest(OpenChannelRequestBuilder builder) { this.defaultTimezone = builder.defaultTimezone; this.offsetToken = builder.offsetToken; this.isOffsetTokenProvided = builder.isOffsetTokenProvided; - this.dropOnClose = builder.dropOnClose; } public String getDBName() { @@ -165,8 +153,4 @@ public String getOffsetToken() { public boolean isOffsetTokenProvided() { return this.isOffsetTokenProvided; } - - public boolean getDropOnClose() { - return this.dropOnClose; - } } diff --git a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestChannel.java b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestChannel.java index a8bb5db16..72004554d 100644 --- a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestChannel.java +++ b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestChannel.java @@ -79,6 +79,14 @@ public interface SnowflakeStreamingIngestChannel { */ CompletableFuture close(); + /** + * Close the channel, this function will make sure all the data in this channel is committed + * + * @param drop if true, the channel will be dropped after all data is successfully committed. + * @return a completable future which will be completed when the channel is closed + */ + CompletableFuture close(boolean drop); + /** * Insert one row into the channel, the row is represented using Map where the key is column name * and the value is a row of data. The following table summarizes supported value types and their diff --git a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClient.java b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClient.java index 3054d9564..8b65b5cae 100644 --- a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClient.java +++ b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClient.java @@ -28,12 +28,10 @@ public interface SnowflakeStreamingIngestClient extends AutoCloseable { /** * Drop the specified channel on the server using a {@link DropChannelRequest} * - *

Note that {@link DropChannelRequest.DropChannelRequestBuilder#setClientSequencer(Long)}} can - * be used to drop a specific version of the channel and prevent accidentally dropping a channel - * concurrently opened by another client. If it is not specified, this call will blindly drop the - * latest version of the channel and any pending data will be lost. Also see {@link - * OpenChannelRequest.OpenChannelRequestBuilder#setDropOnClose(boolean)} to automatically drop - * channels on close. + *

Note that this call will blindly drop the latest version of the channel and any pending data + * will be lost. Also see {@link SnowflakeStreamingIngestChannel#close(boolean)} to drop channels + * on close. That approach will drop the local version of the channel and if the channel has been + * concurrently reopened by another client, that version of the channel won't be affected. * * @param request the drop channel request */ diff --git a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java index 9ba87cffb..ecab62432 100644 --- a/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java +++ b/src/main/java/net/snowflake/ingest/streaming/SnowflakeStreamingIngestClientFactory.java @@ -6,7 +6,6 @@ import java.util.Map; import java.util.Properties; - import net.snowflake.ingest.streaming.internal.SnowflakeStreamingIngestClientInternal; import net.snowflake.ingest.utils.Constants; import net.snowflake.ingest.utils.SnowflakeURL; @@ -71,10 +70,10 @@ public SnowflakeStreamingIngestClient build() { if (addAccountNameInRequest) { return new SnowflakeStreamingIngestClientInternal<>( - this.name, accountURL, prop, this.parameterOverrides, addAccountNameInRequest); + this.name, accountURL, prop, this.parameterOverrides, addAccountNameInRequest); } return new SnowflakeStreamingIngestClientInternal<>( - this.name, accountURL, prop, this.parameterOverrides); + this.name, accountURL, prop, this.parameterOverrides); } } } diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelResponse.java b/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelResponse.java index e36692915..fce91f1d2 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelResponse.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelResponse.java @@ -1,3 +1,7 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + package net.snowflake.ingest.streaming.internal; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelVersionRequest.java b/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelVersionRequest.java new file mode 100644 index 000000000..9afdbcbb3 --- /dev/null +++ b/src/main/java/net/snowflake/ingest/streaming/internal/DropChannelVersionRequest.java @@ -0,0 +1,19 @@ +package net.snowflake.ingest.streaming.internal; + +import net.snowflake.ingest.streaming.DropChannelRequest; + +/** + * Same as DropChannelRequest but allows specifying a client sequencer to drop a specific version. + */ +class DropChannelVersionRequest extends DropChannelRequest { + private final Long clientSequencer; + + public DropChannelVersionRequest(DropChannelRequestBuilder builder, long clientSequencer) { + super(builder); + this.clientSequencer = clientSequencer; + } + + Long getClientSequencer() { + return this.clientSequencer; + } +} diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelFactory.java b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelFactory.java index ace1c80c8..3e442d43b 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelFactory.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelFactory.java @@ -30,8 +30,6 @@ static class SnowflakeStreamingIngestChannelBuilder { private ZoneId defaultTimezone; - private boolean dropOnClose; - private SnowflakeStreamingIngestChannelBuilder(String name) { this.name = name; } @@ -93,11 +91,6 @@ SnowflakeStreamingIngestChannelBuilder setOwningClient( return this; } - SnowflakeStreamingIngestChannelBuilder setDropOnClose(boolean dropOnClose) { - this.dropOnClose = dropOnClose; - return this; - } - SnowflakeStreamingIngestChannelInternal build() { Utils.assertStringNotNullOrEmpty("channel name", this.name); Utils.assertStringNotNullOrEmpty("table name", this.tableName); @@ -123,8 +116,7 @@ SnowflakeStreamingIngestChannelInternal build() { this.encryptionKeyId, this.onErrorOption, this.defaultTimezone, - this.owningClient.getParameterProvider().getBlobFormatVersion(), - this.dropOnClose); + this.owningClient.getParameterProvider().getBlobFormatVersion()); } } } diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelInternal.java b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelInternal.java index 08e30892a..0948abe4e 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelInternal.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelInternal.java @@ -43,7 +43,6 @@ class SnowflakeStreamingIngestChannelInternal implements SnowflakeStreamingIn // Reference to the row buffer private final RowBuffer rowBuffer; - private final boolean dropOnClose; // Indicates whether the channel is closed private volatile boolean isClosed; @@ -81,8 +80,7 @@ class SnowflakeStreamingIngestChannelInternal implements SnowflakeStreamingIn String encryptionKey, Long encryptionKeyId, OpenChannelRequest.OnErrorOption onErrorOption, - ZoneOffset defaultTimezone, - boolean dropOnClose) { + ZoneOffset defaultTimezone) { this( name, dbName, @@ -96,8 +94,7 @@ class SnowflakeStreamingIngestChannelInternal implements SnowflakeStreamingIn encryptionKeyId, onErrorOption, defaultTimezone, - client.getParameterProvider().getBlobFormatVersion(), - dropOnClose); + client.getParameterProvider().getBlobFormatVersion()); } /** Default constructor */ @@ -114,8 +111,7 @@ class SnowflakeStreamingIngestChannelInternal implements SnowflakeStreamingIn Long encryptionKeyId, OpenChannelRequest.OnErrorOption onErrorOption, ZoneId defaultTimezone, - Constants.BdecVersion bdecVersion, - boolean dropOnClose) { + Constants.BdecVersion bdecVersion) { this.isClosed = false; this.owningClient = client; this.channelFlushContext = @@ -132,7 +128,6 @@ class SnowflakeStreamingIngestChannelInternal implements SnowflakeStreamingIn channelState, new ClientBufferParameters(owningClient)); this.tableColumns = new HashMap<>(); - this.dropOnClose = dropOnClose; logger.logInfo( "Channel={} created for table={}", this.channelFlushContext.getName(), @@ -272,6 +267,11 @@ CompletableFuture flush(boolean closing) { */ @Override public CompletableFuture close() { + return this.close(false); + } + + @Override + public CompletableFuture close(boolean drop) { checkValidation(); if (isClosed()) { @@ -297,18 +297,14 @@ public CompletableFuture close() { .map(SnowflakeStreamingIngestChannelInternal::getFullyQualifiedName) .collect(Collectors.toList())); } - if (this.dropOnClose) { - this.owningClient.dropChannel( + if (drop) { + DropChannelRequest.DropChannelRequestBuilder builder = DropChannelRequest.builder(this.getChannelContext().getName()) .setDBName(this.getDBName()) .setTableName(this.getTableName()) - .setSchemaName(this.getSchemaName()) - .setClientSequencer(this.getChannelSequencer()) - .build()); - System.out.println( - "SUCCESSFULLY dropped " - + this.getChannelContext().getFullyQualifiedName() - + " channel"); + .setSchemaName(this.getSchemaName()); + this.owningClient.dropChannel( + new DropChannelVersionRequest(builder, this.getChannelSequencer())); } }); } diff --git a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java index dd9d24e36..767acf3e1 100644 --- a/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java +++ b/src/main/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientInternal.java @@ -217,7 +217,8 @@ public class SnowflakeStreamingIngestClientInternal implements SnowflakeStrea prop.getProperty(Constants.OAUTH_CLIENT_SECRET), prop.getProperty(Constants.OAUTH_REFRESH_TOKEN)); } - this.requestBuilder = new RequestBuilder( + this.requestBuilder = + new RequestBuilder( accountURL, prop.get(USER).toString(), credential, @@ -278,14 +279,7 @@ public SnowflakeStreamingIngestClientInternal( Properties prop, Map parameterOverrides, boolean addAccountNameInRequest) { - this(name, - accountURL, - prop, - null, - false, - null, - parameterOverrides, - addAccountNameInRequest); + this(name, accountURL, prop, null, false, null, parameterOverrides, addAccountNameInRequest); } /** @@ -404,7 +398,6 @@ public SnowflakeStreamingIngestChannelInternal openChannel(OpenChannelRequest .setEncryptionKeyId(response.getEncryptionKeyId()) .setOnErrorOption(request.getOnErrorOption()) .setDefaultTimezone(request.getDefaultTimezone()) - .setDropOnClose(request.getDropOnClose()) .build(); // Setup the row buffer schema @@ -440,8 +433,12 @@ public void dropChannel(DropChannelRequest request) { payload.put("database", request.getDBName()); payload.put("schema", request.getSchemaName()); payload.put("role", this.role); - if (request.getClientSequencer() != null) { - payload.put("client_sequencer", request.getClientSequencer()); + Long clientSequencer = null; + if (request instanceof DropChannelVersionRequest) { + clientSequencer = ((DropChannelVersionRequest) request).getClientSequencer(); + if (clientSequencer != null) { + payload.put("client_sequencer", clientSequencer); + } } DropChannelResponse response = @@ -469,11 +466,11 @@ public void dropChannel(DropChannelRequest request) { "Drop channel request succeeded, channel={}, table={}, clientSequencer={} client={}", request.getChannelName(), request.getFullyQualifiedTableName(), - request.getClientSequencer(), + clientSequencer, getName()); } catch (IOException | IngestResponseException e) { - throw new SFException(e, ErrorCode.OPEN_CHANNEL_FAILURE, e.getMessage()); + throw new SFException(e, ErrorCode.DROP_CHANNEL_FAILURE, e.getMessage()); } } diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/ChannelCacheTest.java b/src/test/java/net/snowflake/ingest/streaming/internal/ChannelCacheTest.java index 089e66437..ea88d618c 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/ChannelCacheTest.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/ChannelCacheTest.java @@ -38,8 +38,7 @@ public void setup() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); channel2 = new SnowflakeStreamingIngestChannelInternal<>( "channel2", @@ -53,8 +52,7 @@ public void setup() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); channel3 = new SnowflakeStreamingIngestChannelInternal<>( "channel3", @@ -68,8 +66,7 @@ public void setup() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); cache.addChannel(channel1); cache.addChannel(channel2); cache.addChannel(channel3); @@ -95,8 +92,7 @@ public void testAddChannel() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); cache.addChannel(channel); Assert.assertEquals(1, cache.getSize()); Assert.assertTrue(channel == cache.iterator().next().getValue().get(channelName)); @@ -114,8 +110,7 @@ public void testAddChannel() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); cache.addChannel(channelDup); // The old channel should be invalid now Assert.assertTrue(!channel.isValid()); @@ -196,8 +191,7 @@ public void testRemoveChannel() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); cache.removeChannelIfSequencersMatch(channel3Dup); // Verify that remove the same channel with a different channel sequencer is a no op Assert.assertEquals(1, cache.getSize()); diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/FlushServiceTest.java b/src/test/java/net/snowflake/ingest/streaming/internal/FlushServiceTest.java index 7c0eb86fb..17f929206 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/FlushServiceTest.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/FlushServiceTest.java @@ -258,8 +258,7 @@ SnowflakeStreamingIngestChannelInternal>> createChannel( encryptionKeyId, onErrorOption, defaultTimezone, - Constants.BdecVersion.THREE, - false); + Constants.BdecVersion.THREE); } @Override @@ -875,8 +874,7 @@ public void testInvalidateChannels() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - ZoneOffset.UTC, - false); + ZoneOffset.UTC); SnowflakeStreamingIngestChannelInternal channel2 = new SnowflakeStreamingIngestChannelInternal<>( @@ -891,8 +889,7 @@ public void testInvalidateChannels() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - ZoneOffset.UTC, - false); + ZoneOffset.UTC); channelCache.addChannel(channel1); channelCache.addChannel(channel2); diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelTest.java b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelTest.java index 0a0a2124b..bfb741bab 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelTest.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestChannelTest.java @@ -33,7 +33,6 @@ import net.snowflake.client.jdbc.internal.apache.http.impl.client.CloseableHttpClient; import net.snowflake.ingest.TestUtils; import net.snowflake.ingest.connection.RequestBuilder; -import net.snowflake.ingest.streaming.DropChannelRequest; import net.snowflake.ingest.streaming.InsertValidationResponse; import net.snowflake.ingest.streaming.OpenChannelRequest; import net.snowflake.ingest.streaming.SnowflakeStreamingIngestChannel; @@ -173,8 +172,7 @@ public void testChannelValid() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); Assert.assertTrue(channel.isValid()); channel.invalidate("from testChannelValid"); @@ -224,8 +222,7 @@ public void testChannelClose() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); Assert.assertFalse(channel.isClosed()); channel.markClosed(); @@ -552,8 +549,7 @@ public void testInsertRow() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); ColumnMetadata col = new ColumnMetadata(); col.setName("COL"); @@ -634,8 +630,7 @@ public void testInsertTooLargeRow() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); channel.setupSchema(schema); InsertValidationResponse insertValidationResponse = channel.insertRow(row, "token-1"); @@ -659,8 +654,7 @@ public void testInsertTooLargeRow() { "key", 1234L, OpenChannelRequest.OnErrorOption.ABORT, - UTC, - false); + UTC); channel.setupSchema(schema); try { @@ -685,8 +679,7 @@ public void testInsertTooLargeRow() { "key", 1234L, OpenChannelRequest.OnErrorOption.SKIP_BATCH, - UTC, - false); + UTC); channel.setupSchema(schema); insertValidationResponse = channel.insertRow(row, "token-1"); @@ -719,8 +712,7 @@ public void testInsertRowThrottling() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); ParameterProvider parameterProvider = new ParameterProvider(); memoryInfoProvider.freeMemory = @@ -766,8 +758,7 @@ public void testFlush() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); ChannelsStatusResponse response = new ChannelsStatusResponse(); response.setStatusCode(0L); response.setMessage("Success"); @@ -803,8 +794,7 @@ public void testClose() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); ChannelsStatusResponse response = new ChannelsStatusResponse(); response.setStatusCode(0L); response.setMessage("Success"); @@ -838,8 +828,7 @@ public void testDropOnClose() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - true); + UTC); ChannelsStatusResponse response = new ChannelsStatusResponse(); response.setStatusCode(0L); response.setMessage("Success"); @@ -848,18 +837,49 @@ public void testDropOnClose() throws Exception { Mockito.doReturn(response).when(client).getChannelsStatus(Mockito.any()); Assert.assertFalse(channel.isClosed()); - DropChannelResponse dropChannelResponse = new DropChannelResponse(); Mockito.doNothing().when(client).dropChannel(Mockito.any()); - channel.close().get(); + channel.close(true).get(); Assert.assertTrue(channel.isClosed()); Mockito.verify(client, Mockito.times(1)) .dropChannel( argThat( - (DropChannelRequest req) -> + (DropChannelVersionRequest req) -> req.getChannelName().equals(channel.getName()) && req.getClientSequencer().equals(channel.getChannelSequencer()))); } + @Test + public void testDropOnCloseInvalidChannel() throws Exception { + SnowflakeStreamingIngestClientInternal client = + Mockito.spy(new SnowflakeStreamingIngestClientInternal<>("client")); + SnowflakeStreamingIngestChannelInternal channel = + new SnowflakeStreamingIngestChannelInternal<>( + "channel", + "db", + "schema", + "table", + "0", + 1L, + 0L, + client, + "key", + 1234L, + OpenChannelRequest.OnErrorOption.CONTINUE, + UTC); + ChannelsStatusResponse response = new ChannelsStatusResponse(); + response.setStatusCode(0L); + response.setMessage("Success"); + response.setChannels(new ArrayList<>()); + + Mockito.doReturn(response).when(client).getChannelsStatus(Mockito.any()); + + Assert.assertFalse(channel.isClosed()); + channel.invalidate("test"); + Mockito.doNothing().when(client).dropChannel(Mockito.any()); + Assert.assertThrows(SFException.class, () -> channel.close(true).get()); + Mockito.verify(client, Mockito.never()).dropChannel(Mockito.any()); + } + @Test public void testGetLatestCommittedOffsetToken() { String offsetToken = "10"; @@ -878,8 +898,7 @@ public void testGetLatestCommittedOffsetToken() { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); ChannelsStatusResponse response = new ChannelsStatusResponse(); response.setStatusCode(0L); diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java index c956529e6..1039a5228 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/SnowflakeStreamingIngestClientTest.java @@ -1,7 +1,16 @@ package net.snowflake.ingest.streaming.internal; import static java.time.ZoneOffset.UTC; -import static net.snowflake.ingest.utils.Constants.*; +import static net.snowflake.ingest.utils.Constants.ACCOUNT_URL; +import static net.snowflake.ingest.utils.Constants.CHANNEL_STATUS_ENDPOINT; +import static net.snowflake.ingest.utils.Constants.DROP_CHANNEL_ENDPOINT; +import static net.snowflake.ingest.utils.Constants.MAX_STREAMING_INGEST_API_CHANNEL_RETRY; +import static net.snowflake.ingest.utils.Constants.PRIVATE_KEY; +import static net.snowflake.ingest.utils.Constants.REGISTER_BLOB_ENDPOINT; +import static net.snowflake.ingest.utils.Constants.RESPONSE_ERR_ENQUEUE_TABLE_CHUNK_QUEUE_FULL; +import static net.snowflake.ingest.utils.Constants.RESPONSE_SUCCESS; +import static net.snowflake.ingest.utils.Constants.ROLE; +import static net.snowflake.ingest.utils.Constants.USER; import static net.snowflake.ingest.utils.ParameterProvider.ENABLE_SNOWPIPE_STREAMING_METRICS; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; @@ -88,8 +97,7 @@ public void setup() { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); channel2 = new SnowflakeStreamingIngestChannelInternal<>( "channel2", @@ -104,8 +112,7 @@ public void setup() { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); channel3 = new SnowflakeStreamingIngestChannelInternal<>( "channel3", @@ -120,8 +127,7 @@ public void setup() { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); channel4 = new SnowflakeStreamingIngestChannelInternal<>( "channel4", @@ -136,8 +142,7 @@ public void setup() { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); } @Test @@ -353,8 +358,7 @@ public void testGetChannelsStatusWithRequest() throws Exception { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); ChannelsStatusRequest.ChannelStatusRequestDTO dto = new ChannelsStatusRequest.ChannelStatusRequestDTO(channel); @@ -458,8 +462,7 @@ public void testGetChannelsStatusWithRequestError() throws Exception { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); try { client.getChannelsStatus(Collections.singletonList(channel)); @@ -500,8 +503,7 @@ public void testRegisterBlobRequestCreationSuccess() throws Exception { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); ChannelMetadata channelMetadata = ChannelMetadata.builder() @@ -1188,8 +1190,7 @@ public void testRegisterBlobResponseWithInvalidChannel() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); SnowflakeStreamingIngestChannelInternal channel2 = new SnowflakeStreamingIngestChannelInternal<>( channel2Name, @@ -1203,8 +1204,7 @@ public void testRegisterBlobResponseWithInvalidChannel() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); client.getChannelCache().addChannel(channel1); client.getChannelCache().addChannel(channel2); @@ -1331,8 +1331,7 @@ public void testVerifyChannelsAreFullyCommittedSuccess() throws Exception { "key", 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, - UTC, - false); + UTC); client.getChannelCache().addChannel(channel); ChannelsStatusResponse response = new ChannelsStatusResponse(); @@ -1420,8 +1419,7 @@ public void testGetLatestCommittedOffsetTokens() throws Exception { 1234L, OpenChannelRequest.OnErrorOption.CONTINUE, ZoneOffset.UTC, - BDEC_VERSION, - false); + BDEC_VERSION); ChannelsStatusRequest.ChannelStatusRequestDTO dto = new ChannelsStatusRequest.ChannelStatusRequestDTO(channel); diff --git a/src/test/java/net/snowflake/ingest/streaming/internal/StreamingIngestIT.java b/src/test/java/net/snowflake/ingest/streaming/internal/StreamingIngestIT.java index 33cfb5e51..8257d9bf8 100644 --- a/src/test/java/net/snowflake/ingest/streaming/internal/StreamingIngestIT.java +++ b/src/test/java/net/snowflake/ingest/streaming/internal/StreamingIngestIT.java @@ -206,18 +206,17 @@ public void testDropChannel() throws Exception { .setSchemaName(TEST_SCHEMA) .setTableName(TEST_TABLE) .setOnErrorOption(OpenChannelRequest.OnErrorOption.CONTINUE) - .setDropOnClose(true) .build(); // Open a streaming ingest channel from the given client SnowflakeStreamingIngestChannel channel1 = client.openChannel(request1); // Close the channel after insertion - channel1.close().get(); + channel1.close(true).get(); // verify expected request sent to server Mockito.verify(requestBuilder) .generateStreamingIngestPostRequest( - ArgumentMatchers.contains("channel"), + ArgumentMatchers.contains("client_sequencer"), ArgumentMatchers.refEq(DROP_CHANNEL_ENDPOINT), ArgumentMatchers.refEq("drop channel")); }