SNOW-1061848: Introduce TopicPartitionChannel interface

[sfc-gh-akowalczyk]

Overview

SNOW-1061848

We want to remove double buffering from the project completely, which is quite a significant change, so we need to temporarily support running with and without the additional buffer.

This PR is a first attempt to extract an interface that both approaches (with and without buffer) will implement so we can easily switch between them.

Pre-review checklist

• This change should be part of a Behavior Change Release. See go/behavior-change.
• This change has passed Merge gate tests
• Snowpipe Changes
• Snowpipe Streaming Changes
• This change is TEST-ONLY
• This change is README/Javadocs only
• This change is protected by a config parameter <PARAMETER_NAME> eg snowflake.ingestion.method.
  • Yes - Added end to end and Unit Tests.
  • No - Suggest why it is not param protected
• Is his change protected by parameter <PARAMETER_NAME> on the server side?
  • The parameter/feature is not yet active in production (partial rollout or PrPr, see Changes for Unreleased Features and Fixes).
  • If there is an issue, it can be safely mitigated by turning the parameter off. This is also verified by a test (See go/ppp).

[sfc-gh-dseweryn]

Generally OK for me. I think that the ExposingInternalsTopicPartition interface should be either directly implemented on a particular TopicPartitionChannel implementation or available only in tests

[sfc-gh-akowalczyk]

To avoid confusion, the intention was to create this interface as a midstep before investing time in further refactoring. These methods existed already in the previous implementation and were freely accessible by other classes (used to have protected modifier).

In the future, I'd like to get rid of the interface but for now, I'd like to focus on discussing whether having a single interface TopicPartitionChannel with 2 implementations (buffered and non-buffered) is the way to go.

The idea is to quickly create a space for a new implementation - DirectTopicPartitionChannel and implement it.

Additional refactoring will be done afterward.

[sfc-gh-mbobowski]

I tried to imagine how I would approach this problem and I was thinking about:

1. Creating a builder for this class to have only one constructo
2. Creating an abstract class instead of interface. Abstract class would contain channel and all methods that are not related to buffering (like getOffsetSafeToCommitToKafka, closeChannel, isChannelClose, getChannelNameFormat).

But I am also ok with the interface-based approach.

[sfc-gh-akowalczyk]

I tried to imagine how I would approach this problem and I was thinking about:

1. Creating a builder for this class to have only one constructo
2. Creating an abstract class instead of interface. Abstract class would contain channel and all methods that are not related to buffering (like getOffsetSafeToCommitToKafka, closeChannel, isChannelClose, getChannelNameFormat).

But I am also ok with the interface-based approach.

Thanks, let me rethink this approach.

The interface can be easily replaced with an abstract class, so I'll merge it as it is now. In one of the next iterations it can be considered for replace.

[sfc-gh-gjachimko]

do we really need to change visibility of these methods?

[sfc-gh-gjachimko]

ok, i saw the other interface few lines down..

[sfc-gh-gjachimko]

this probably doesn't hurt, as it isn't used anywhere, but i tent do throw UnsupportedOperationException for such initial stub methods.

[sfc-gh-akowalczyk]

let me leave it as it is now, it will be replaced in the next iterations

[sfc-gh-gjachimko]

lgtm. solid baseline for further work. i like the name refactor.