Search code examples
javamultithreadingpriority-queuejava.util.concurrentblockingqueue

DelayQueue unexpected behavior. DrainTo deletes from the queue only 1 expired item


I want to iterate through unexpired elements in my DelayQueue. The class Transaction implements Delayed, and has a field timestamp, which represents the timestamp of a transaction when it was originated in UTC (not the current timestamp)

public class Transaction implements Delayed {
    private final Double amount;
    private final Long timestamp;   //timestamp of a time when the item was created and send here


    public Transaction(double amount, long timestamp) {
        this.amount = amount;
        this.timestamp = timestamp;
    }

    @Override
    public long getDelay(TimeUnit unit) {
        long delay = unit.convert(ONEMINUTE - (System.currentTimeMillis() - timestamp), TimeUnit.MILLISECONDS);
    return delay;
}

    @Override
    public int compareTo(Delayed delayed) {
        if (delayed == this) {
            return 0;
        }

        if (delayed instanceof Transaction) {
            return 0;
        }

        long diff = (getDelay(TimeUnit.MILLISECONDS) - delayed.getDelay(TimeUnit.MILLISECONDS));
        return ((diff == 0) ? 0 : ((diff < 0) ? -1 : 1));
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + timestamp.hashCode();
        return result;
    }

    @Override
    public boolean equals( Object obj ) {
        if( this == obj ) {
            return true;
        }

        if( obj == null ) {
            return false;
        }

        if( !( obj instanceof Transaction ) ) {
            return false;
        }

        final Transaction other = ( Transaction )obj;
        return timestamp.equals(other.timestamp);
    }
}

The TransactionManager class below adds the incoming transaction to the queue, if the new coming transaction is younger than 1 minute. On getStatistics, the old transactions should be removed from the queue, and the queue should contain only transactions younger then 1 minute

public class TransactionManager {

    private DelayQueue<Transaction> transactions;


    public TransactionManager() {
        transactions = new DelayQueue<>();
        System.setProperty("user.timezone", "UTC");
    }

    public Object createTransaction(String json) {
        JSONObject jsonObject = null;
        try {
            jsonObject = JsonValidator.validateTransactionJson(json);
        } catch (Exception ex) {
            return new ResponseEntity(HttpStatus.UNPROCESSABLE_ENTITY);
        }
        long delay = System.currentTimeMillis() - ((Long) jsonObject.get(TIMESTAMP));
        if (delay > ONEMINUTE) {
            return new ResponseEntity(HttpStatus.NO_CONTENT);
        }
        transactions.add(new Transaction((Double) jsonObject.get(AMOUNT), (Long) jsonObject.get(TIMESTAMP)));
        return new ResponseEntity(HttpStatus.OK);
    }

    public long getStatistics() {
        List<Transaction> tempForCleaning = new ArrayList<>();
        transactions.drainTo(tempForCleaning);
        tempForCleaning.clear();
        StatisticJSON statistics = new StatisticJSON();
        transactions.stream().forEach(transaction -> {
            statistics.setCount(statistics.getCount() + 1);
        });
        return statistics.getCount();
    }
}

In this test I create 5 transactions with the time of 40 seconds before now, and 3 transactions of 10 seconds before now. So after waiting 45 seconds, the first 5 should be drained, and the queue should contain only 3 transaction, however, the method drainTo removes only 1 old transaction.

@Test
public void test() {
    DateTime dateTime = new DateTime(DateTimeZone.UTC);
    long fortyMilliSecondsAgo = dateTime.minusSeconds(40).getMillis();
    long twentyMilliSecondsAgo = dateTime.minusSeconds(10).getMillis();
    for (int i = 0; i < 5; i++) {
        createTransaction(fortyMilliSecondsAgo);
    }
    for (int i = 0; i < 3; i++) {
        createTransaction(twentyMilliSecondsAgo);
    }
    Assert.assertTrue(transactionManager.getStatistics() == 8);
    try {
        TimeUnit.SECONDS.sleep(45);
        System.out.println("\n\n\n");
        Assert.assertTrue(transactionManager.getStatistics() == 3);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

private void createTransaction(long timestamp) {
    transactionManager.createTransaction("{\"amount\":100.0,\"timestamp\":" + timestamp + "}");
}

I'm missing something, why drainTo removes only one expired item, even there is 4 left, but cannot catch where...


Solution

  • if (delayed instanceof Transaction) { return 0; }
    

    doesn't look right - you should probably remove that bit if you want compareTo to be consistent with getDelay(). So your method should probably look like this (with static imports):

    public int compareTo(Delayed delayed) {
      return delayed == this
        ? 0
        : Long.compare(getDelay(MILLISECONDS), delayed.getDelay(MILLISECONDS));
    }