Allow clients to drop channel on close or blindly

[sfc-gh-psaha]

This change exposes a new API to drop channels. It can be done either on channel close (verifies that no other client has raced to reopen the channel) or blindly with just a table name.
Note that dropping a channel blindly discards any pending data and will invalidate any already opened channel - so use with caution.

[sfc-gh-psaha]

Note that tests will fail until the API is enabled on the server side.

[sfc-gh-xhuang]

@sfc-gh-psaha do we want to allow this only on primary or both primary and secondary?
I think primary only for replication is standard as we disallow writes and deletes on secondary
https://docs.snowflake.com/en/user-guide/account-replication-considerations#replication-and-snowpipe-streaming

[sfc-gh-psaha]

@sfc-gh-psaha do we want to allow this only on primary or both primary and secondary? I think primary only for replication is standard as we disallow writes and deletes on secondary https://docs.snowflake.com/en/user-guide/account-replication-considerations#replication-and-snowpipe-streaming

Currently, we allow it only on the primary - same as open channel. This check is performed server-side.

[sfc-gh-tzhang]

Thanks for the change! My biggest discussion point is how to expose the API to the customer. To me, it seems more intuitive to support a new channel.close(boolean ifDrop) function instead of specifying it as part of open channel, WDYT? I'm not sure if we have ask this to the customers that requested this feature

[sfc-gh-tzhang]

Another option is to support another .close() which takes this as an input, which one do you think it's better?

[sfc-gh-psaha]

The original reason I did it this way was because I thought Channel extends AutoCloseable and therefore, we cannot change the signature of close but I was mistaken - only the client extends it. Since users of this class has to call close explicitly anyways, I think your suggestions is the less-intrusive approach. There is still a corner case of someone using try-with using the Client and closing channels that way implicitly without any option to drop them but I think that's likely a rare corner case. We can always go and implement my idea if that is requested.

[sfc-gh-tzhang]

DROP_CHANNEL_FAILURE

[sfc-gh-tzhang]

should we drop the channel even if it hits the exception above?

[sfc-gh-tzhang]

In theory, I don't see an issue of dropping a closed channel, should this be removed?

[sfc-gh-tzhang]

Also what about when a channel becomes invalid, currently we can't close an invalid channel, but we should allow dropping them?

[sfc-gh-psaha]

I think dropping after failing to commit would be a surprising behavior. Typically, when close fails, the caller is supposed to handle it e.g. by retrying etc. Dropping in this specific case would lose uncommitted data.

[sfc-gh-tzhang]

I think it depends, I can imagine a use case where customer always open a new channel with an unique identifier every time the old channel gets invalidated. I guess we can come back to this if needed in the future, basically right now we won't drop any closed and invalid channels, better to add it somewhere in the comment.

[sfc-gh-tzhang]

LGTM, thanks!

[sfc-gh-xhuang]

let me know if this is ready

[sfc-gh-psaha]

@sfc-gh-tzhang Please take another look.

[sfc-gh-tzhang]

LGTM, thanks!

[sfc-gh-tzhang]

The change itself looks good to me, it's just the impact of accidentally dropping the wrong channel worries me as the offset token info will be lost. We need to be careful about documenting this, maybe this is something customer wants to reach out to support before doing this. cc: @sfc-gh-xhuang