jonathan-lemos commented on code in PR #33057:
URL: https://github.com/apache/beam/pull/33057#discussion_r1838371628
##########
runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/StringSetCellTest.java:
##########
@@ -94,4 +99,37 @@ public void testReset() {
assertThat(stringSetCell.getCumulative(), equalTo(StringSetData.empty()));
assertThat(stringSetCell.getDirty(), equalTo(new DirtyState()));
}
+
+ @Test(timeout = 5000)
+ public void testStringSetCellConcurrentAddRetrivial() throws
InterruptedException {
+ StringSetCell cell = new StringSetCell(MetricName.named("namespace",
"name"));
+ AtomicBoolean finished = new AtomicBoolean(false);
+ Thread increment =
+ new Thread(
+ () -> {
+ for (long i = 0; !finished.get(); ++i) {
+ cell.add(String.valueOf(i));
+ try {
+ Thread.sleep(1);
+ } catch (InterruptedException e) {
+ break;
+ }
+ }
+ });
+ increment.start();
+ Instant start = Instant.now();
+ try {
+ while (true) {
+ Set<String> s = cell.getCumulative().stringSet();
+ List<String> snapshot = new ArrayList<>(s);
+ if (Instant.now().isAfter(start.plusSeconds(3)) && snapshot.size() >
0) {
Review Comment:
We should assert that the contents of the snapshot are not corrupt.
Maybe we keep track of the highest integer reached by the thread, and assert
that at the end, the snapshot contains all of [0, N].
##########
runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/StringSetCellTest.java:
##########
@@ -94,4 +99,37 @@ public void testReset() {
assertThat(stringSetCell.getCumulative(), equalTo(StringSetData.empty()));
assertThat(stringSetCell.getDirty(), equalTo(new DirtyState()));
}
+
+ @Test(timeout = 5000)
+ public void testStringSetCellConcurrentAddRetrivial() throws
InterruptedException {
Review Comment:
Nit: `s/Retrivial/Retrieval`
##########
runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/StringSetCellTest.java:
##########
@@ -94,4 +99,37 @@ public void testReset() {
assertThat(stringSetCell.getCumulative(), equalTo(StringSetData.empty()));
assertThat(stringSetCell.getDirty(), equalTo(new DirtyState()));
}
+
+ @Test(timeout = 5000)
+ public void testStringSetCellConcurrentAddRetrivial() throws
InterruptedException {
+ StringSetCell cell = new StringSetCell(MetricName.named("namespace",
"name"));
+ AtomicBoolean finished = new AtomicBoolean(false);
+ Thread increment =
+ new Thread(
+ () -> {
+ for (long i = 0; !finished.get(); ++i) {
+ cell.add(String.valueOf(i));
+ try {
+ Thread.sleep(1);
Review Comment:
1s sleep in a 3s test doesn't give much room to find races.
Does this test fail without your changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
