KAFKA-8006: Guard calls to init and close from global processor

[ableegoldman]

Guards against users calling init() and close() on global stores accessed via global processor

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[ableegoldman]

@mjsax @guozhangwang @bbejeck @vvcephei

[bbejeck]

LGTM

[guozhangwang]

LGTM.

[mjsax]

Overall LGTM. One nit for the unit tests.

[mjsax]

This is two methods calls, thus it should not be a one liner (test would pass if getStateStore() would throw UnsupportedOperationException.

Should be:

@Test
    public void shouldNotAllowInit() {
        final StateStore store = globalContext.getStateStore(GLOBAL_STORE_NAME);
        try {
            store.init(null, null);
            fail("Should have thrown UnsupportedOperationException.");
        } catch(final UnsupportedOperationException expected) { }

Similar below.

[ableegoldman]

Ack

[mjsax]

Actually, you should compare the tests in ProcessorContextImplTest -- they are quite sophisticated and it might be worth to replicate them here?

[ableegoldman]

@mjsax Do you mean the tests in general or with regards to init & close specifically? I guess it's not clear to me what benefit the sophistication is really providing here..I felt it was better to do a straightforward test of the single functionality being tested, but am open to doing otherwise

[mjsax]

Also, there is a dependency between this PR and #6173 and #6356. Depending which one we merge first, the other need to be updated.

[bbejeck]

Java 11 failed org.apache.kafka.streams.state.internals.RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.shouldForwardAllDbOptionsCalls
Java 8 failed

kafka.api.AdminClientIntegrationTest.testMinimumRequestTimeouts
kafka.api.ConsumerBounceTest.testRollingBrokerRestartsWithSmallerMaxGroupSizeConfigDisruptsBigGroup

retest this please

[mjsax]

@ableegoldman The fancy test also verifies that only init() and close() are guarded and it's written to avoid test code duplication. But you raise a fair point that the fancy test is complex code. But it's already there so it might be worth do "copy and adapt" to the global case, too. Thoughts @guozhangwang @bbejeck? If we want to go with simpler test pattern, it might be worth to rewrite the existing fancy test to simplify it?

[guozhangwang]

I looked at ProcessorContextImplTest and I agree with @ableegoldman that it seems over-complicated, we do not need to follow its pattern and even better, simplify that one as well.

[mjsax]

Thanks for the input @guozhangwang!

Merging this now. We can do follow ups if we want to simplify existing test code.