[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 Manually merged ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 Merging this after fixing the comment... ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 @zentol Have a look at the way I solved that above, see if you agree that we covered our bases now. ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 I added a way to reset the flag into its original state (prior to assertion activation) and used that to test that the concurrency check is not active by default. The `KryoSerializerConcurrencyTest` tests the default test setting (concurrency checks on), the `KryoSerializerConcurrencyCheckInactiveITCase` tests the general default (concurrency checks off). ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 Ah, I see. Let me think whether there is an easy way to do that... ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5645 The end-to-end test would be cheap as we wouldn't execute a flink job or even start a flink cluster. The scripts in `flink-end-to-end-tests` can do pretty much anything they want; my idea was to just call the test method in a non-test context, in which case the test should throw an AssertionError. It may not be the cleanest thing to do though as we have to max test and production code in a single jar. My goal was to verify that the sections guarded by `CONCURRENT_ACCESS_CHECK` are skipped at runtime by default. As it stands someone could just hard-code it to `true` and there's no test preventing that. ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 Making the concurrent access check re-entrant fixes all tests. My feeling would be to not add an end-to-end test for this, because end-to-end tests are quite expensive. Is this mainly for validating that this works by configuring the log level in your opinion? ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5645 Thanks for the great review. Good catch, I bet the missing check against the current thread is the reason for the test failure. ---
[GitHub] flink issue #5645: FLINK-8876 Improve concurrent access handling in stateful...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5645 We got a bunch of failing tests: ``` Failed tests: MultidimensionalArraySerializerTest.testObjectArrays:84 Exception in test: Serializer already accessed by thread main PojoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testBeanStyleObjects:95->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main PojoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testCompositeObject:75->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main PojoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testNestedInterfaces:124->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main PojoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testNestedObjects:85->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main PojoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testSimpleTypesObjects:64->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main PojoSubclassSerializerTest>SerializerTestBase.testSerializedCopyAsSequence:402 Exception in test: Serializer already accessed by thread main PojoSubclassSerializerTest>SerializerTestBase.testSerializedCopyIndividually:364 Exception in test: Serializer already accessed by thread main SubclassFromInterfaceSerializerTest>SerializerTestBase.testSerializedCopyAsSequence:402 Exception in test: Serializer already accessed by thread main SubclassFromInterfaceSerializerTest>SerializerTestBase.testSerializedCopyIndividually:364 Exception in test: Serializer already accessed by thread main TupleSerializerTest.testTuple5CustomObjects:215->runTests:229 Exception in test: Serializer already accessed by thread main KryoGenericArraySerializerTest>AbstractGenericArraySerializerTest.testBeanStyleObjects:120->AbstractGenericArraySerializerTest.runTests:152->AbstractGenericArraySerializerTest.runTests:173 Exception in test: Serializer already accessed by thread main KryoGenericArraySerializerTest>AbstractGenericArraySerializerTest.testCompositeObject:93->AbstractGenericArraySerializerTest.runTests:152->AbstractGenericArraySerializerTest.runTests:173 Exception in test: Serializer already accessed by thread main KryoGenericArraySerializerTest>AbstractGenericArraySerializerTest.testNestedObjects:103->AbstractGenericArraySerializerTest.runTests:152->AbstractGenericArraySerializerTest.runTests:173 Exception in test: Serializer already accessed by thread main KryoGenericArraySerializerTest>AbstractGenericArraySerializerTest.testSimpleTypesObjects:82->AbstractGenericArraySerializerTest.runTests:152->AbstractGenericArraySerializerTest.runTests:173 Exception in test: Serializer already accessed by thread main KryoGenericArraySerializerTest>AbstractGenericArraySerializerTest.testString:63->AbstractGenericArraySerializerTest.runTests:152->AbstractGenericArraySerializerTest.runTests:173 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testBeanStyleObjects:95->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testCompositeObject:75->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest.testJavaDequeue:68->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest.testJavaList:50->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest.testJavaSet:59->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testNestedInterfaces:124->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testNestedObjects:85->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testSimpleTypesObjects:64->AbstractGenericTypeSerializerTest.runTests:155 Exception in test: Serializer already accessed by thread main KryoGenericTypeSerializerTest>AbstractGenericTypeSerializerTest.testString:41->AbstractGenericTypeSerializerTest.runTests:155
