flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GJL <...@git.apache.org>
Subject [GitHub] flink pull request #4915: [FLINK-7838] Bunch of hotfixes and fix missing syn...
Date Wed, 01 Nov 2017 10:35:12 GMT
Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4915#discussion_r148227364
  
    --- Diff: flink-connectors/flink-connector-kafka-0.11/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/FlinkKafkaProducer.java
---
    @@ -226,13 +228,19 @@ public int getTransactionCoordinatorId() {
     
     	private void flushNewPartitions() {
     		LOG.info("Flushing new partitions");
    +		enqueueNewPartitions().await();
    +	}
    +
    +	private TransactionalRequestResult enqueueNewPartitions() {
     		Object transactionManager = getValue(kafkaProducer, "transactionManager");
    -		Object txnRequestHandler = invoke(transactionManager, "addPartitionsToTransactionHandler");
    -		invoke(transactionManager, "enqueueRequest", new Class[]{txnRequestHandler.getClass().getSuperclass()},
new Object[]{txnRequestHandler});
    -		TransactionalRequestResult result = (TransactionalRequestResult) getValue(txnRequestHandler,
txnRequestHandler.getClass().getSuperclass(), "result");
    -		Object sender = getValue(kafkaProducer, "sender");
    -		invoke(sender, "wakeup");
    -		result.await();
    +		synchronized (transactionManager) {
    +			Object txnRequestHandler = invoke(transactionManager, "addPartitionsToTransactionHandler");
    --- End diff --
    
    Did you add the `!newPartitionsInTransaction.isEmpty()` check in the end? I couldn't find
it on first glance. 
    
    Regarding tests, if you add the check, would your current test fail? If not, I think the
behaviour isn't properly tested.


---

Mime
View raw message