Relax assertion in testRetentionLeasesSyncOnExpiration #38070
Conversation
The returned retention leases are collected from a HashMap which the insert-order is not reserved. We should relax the order of leases. Closes elastic#37963
dnhatn
added
>test
|
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
I am confused by this change. Starting with line 146, we assert that on the primary, the leases are either empty, or contain a single non-expired retention lease. For the assertion being changed here to be failing, that means that currentRetentionLeases is not empty. By the assertion on line 146, that means that currentRetentionLeases contains a single retention lease, the one non-expired retention lease. Therefore I do not see how changing from contains to containsInAnyOrder helps with that problem. Rather, I think that something else is going on although it is still not clear to me what. Note how the assert busy shows there was another retention lease on the replica, and then eventually none.
|
@dnhatn I think that I have a better version of this test in a PR that I am working on. |
|
@jasontedor yeah, can I close this one? |
|
Closes in favor of #38380. |
The returned retention leases are collected from a HashMap which the
insert-order is not reserved; thus we should relax the order of leases.
Closes #37963