Netflix/servo

`BasicTagList` with same tags fails equality test when created by different threads

jjteo74 opened this issue · 1 comments

I was following the example in registration of metrics, and found that occasionally duplicate monitors are created for the same MonitorConfig; this only happens when there are multiple threads competing to create the first Monitor instance for a given MonitorConfig.

Eventually found that the equals method of MonitorConfig returns false when comparing the tags, even though the list of tags are the same for the MonitorConfig instances being compared.

The following unit test fails 1 out of 5 times on my laptop (MacOS X 10.10.3, Java 1.7.0_75, Servo 0.9.0).

import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;

import java.util.Collections;
import java.util.Set;
import java.util.concurrent.*;

import org.junit.Test;

import com.netflix.servo.tag.BasicTagList;

public class BasicTagListTests {

    @Test
    public void testBasicTagList() throws Exception {
        final int count = 10;
        final CountDownLatch latch = new CountDownLatch(count);
        final Set<BasicTagList> tagLists = Collections
                .newSetFromMap(new ConcurrentHashMap<BasicTagList, Boolean>());

        final CyclicBarrier barrier = new CyclicBarrier(count);

        for (int i = 0; i < count; i++) {
            new Thread(new Runnable() {
                @Override
                public void run() {
                    try {
                        barrier.await();
                        tagLists.add(BasicTagList.of("id", "1", "color",
                                "green"));
                    } catch (Exception e) {
                        e.printStackTrace(System.out);
                    } finally {
                        latch.countDown();
                    }
                }
            }).start();
        }
        latch.await();
        assertThat(tagLists.size(), is(1));
    }

}

Thanks for the detailed bug report and the test case. I've reproduced your problem and created a pull request with a fix: #317