heesung-sn commented on code in PR #18489: URL: https://github.com/apache/pulsar/pull/18489#discussion_r1027225022
########## pulsar-broker/src/test/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelTest.java: ########## @@ -0,0 +1,772 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.loadbalance.extensions.channel; + +import static org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.MAX_CLEAN_UP_DELAY_TIME_IN_SECS; +import static org.apache.pulsar.metadata.api.extended.SessionEvent.ConnectionLost; +import static org.apache.pulsar.metadata.api.extended.SessionEvent.Reconnected; +import static org.apache.pulsar.metadata.api.extended.SessionEvent.SessionLost; +import static org.apache.pulsar.metadata.api.extended.SessionEvent.SessionReestablished; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; +import static org.testng.AssertJUnit.assertNotNull; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.pulsar.broker.PulsarServerException; +import org.apache.pulsar.broker.PulsarService; +import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest; +import org.apache.pulsar.broker.loadbalance.LeaderElectionService; +import org.apache.pulsar.broker.loadbalance.extensions.models.Split; +import org.apache.pulsar.broker.loadbalance.extensions.models.Unload; +import org.apache.pulsar.client.api.Producer; +import org.apache.pulsar.client.api.TypedMessageBuilder; +import org.apache.pulsar.client.impl.TableViewImpl; +import org.apache.pulsar.common.util.collections.ConcurrentOpenHashMap; +import org.apache.pulsar.metadata.api.NotificationType; +import org.apache.pulsar.metadata.api.extended.SessionEvent; +import org.awaitility.Awaitility; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +@Test(groups = "broker") +public class ServiceUnitStateChannelTest extends MockedPulsarServiceBaseTest { + + private PulsarService pulsar1; + private PulsarService pulsar2; + private ServiceUnitStateChannel channel1; + private ServiceUnitStateChannel channel2; + private String lookupServiceAddress1; + private String lookupServiceAddress2; + private String bundle; + + private String bundle1; + private String bundle2; + + @BeforeClass + @Override + protected void setup() throws Exception { + conf.setAllowAutoTopicCreation(true); + super.internalSetup(conf); + + admin.tenants().createTenant("pulsar", createDefaultTenantInfo()); + admin.namespaces().createNamespace("pulsar/system"); + admin.tenants().createTenant("public", createDefaultTenantInfo()); + admin.namespaces().createNamespace("public/default"); + + pulsar1 = pulsar; + pulsar2 = startBrokerWithoutAuthorization(getDefaultConf()); + channel1 = new ServiceUnitStateChannelImpl(pulsar1); + channel1.start(); + channel2 = new ServiceUnitStateChannelImpl(pulsar2); + channel2.start(); + lookupServiceAddress1 = (String) + FieldUtils.readDeclaredField(channel1, "lookupServiceAddress", true); + lookupServiceAddress2 = (String) + FieldUtils.readDeclaredField(channel2, "lookupServiceAddress", true); + + bundle = String.format("%s/%s", "public/default", "0x00000000_0xffffffff"); + bundle1 = String.format("%s/%s", "public/default", "0x00000000_0xfffffff0"); + bundle2 = String.format("%s/%s", "public/default", "0xfffffff0_0xffffffff"); + } + + @BeforeMethod + protected void initTableViews() throws Exception { + cleanTableView(channel1, bundle); + cleanTableView(channel2, bundle); + } + + + @AfterClass + @Override + protected void cleanup() throws Exception { + channel1.close(); + channel2.close(); + pulsar1 = null; + pulsar2.close(); + super.internalCleanup(); + } + + @Test(priority = 0) + public void channelOwnerTest() throws Exception { + var channelOwner1 = channel1.getChannelOwner(); + var channelOwner2 = channel2.getChannelOwner(); + assertEquals(channelOwner1, channelOwner2); + LeaderElectionService leaderElectionService1 = (LeaderElectionService) FieldUtils.readDeclaredField( + channel1, "leaderElectionService", true); + leaderElectionService1.close(); + waitUntilNewChannelOwner(channel2, channelOwner1); + leaderElectionService1.start(); + waitUntilNewChannelOwner(channel1, channelOwner1); + + var newChannelOwner1 = channel1.getChannelOwner(); + var newChannelOwner2 = channel2.getChannelOwner(); + + assertEquals(newChannelOwner1, newChannelOwner2); + assertNotEquals(channelOwner1, newChannelOwner1); + + if (newChannelOwner1.equals(lookupServiceAddress1)) { + assertTrue(channel1.isChannelOwner()); + assertFalse(channel2.isChannelOwner()); + } else { + assertFalse(channel1.isChannelOwner()); + assertTrue(channel2.isChannelOwner()); + } + } + + @Test(priority = 0) + public void channelValidationTest() throws ExecutionException, InterruptedException { + var channel = new ServiceUnitStateChannelImpl(pulsar); + + int errorCnt = validateChannelStart(channel); + assertEquals(6, errorCnt); + ExecutorService executor = Executors.newSingleThreadExecutor(); + Future startFuture = executor.submit(() -> { + try { + channel.start(); + } catch (PulsarServerException e) { + throw new RuntimeException(e); + } + }); + errorCnt = validateChannelStart(channel); + startFuture.get(); + assertTrue(errorCnt > 0); + Future closeFuture = executor.submit(()->{ + try { + channel.close(); + } catch (PulsarServerException e) { + throw new RuntimeException(e); + } + }); + errorCnt = validateChannelStart(channel); + closeFuture.get(); + assertTrue(errorCnt > 0); + errorCnt = validateChannelStart(channel); + assertEquals(6, errorCnt); + } + + private int validateChannelStart(ServiceUnitStateChannelImpl channel) { + int errorCnt = 0; + try { + channel.isChannelOwner(); + } catch (IllegalStateException e) { + errorCnt++; + } + try { + channel.getChannelOwner(); + } catch (IllegalStateException e) { + errorCnt++; + } + try { + channel.getOwnerAsync(bundle); + } catch (IllegalStateException e) { + errorCnt++; + } + try { + channel.publishAssignEventAsync(bundle, lookupServiceAddress1); + } catch (IllegalStateException e) { + errorCnt++; + } + try { + channel.publishUnloadEventAsync( + new Unload(lookupServiceAddress1, bundle, Optional.of(lookupServiceAddress2))); + } catch (IllegalStateException e) { + errorCnt++; + } + try { + channel.publishSplitEventAsync(new Split(bundle, lookupServiceAddress1, Map.of())); + } catch (IllegalStateException e) { + errorCnt++; + } + return errorCnt; + } + + @Test(priority = 1) + public void compactionScheduleTest() throws PulsarServerException { + + channel1.scheduleCompaction(); + + Awaitility.await() + .pollInterval(200, TimeUnit.MILLISECONDS) + .atMost(5, TimeUnit.SECONDS) + .untilAsserted(() -> { // wait until true + try { + var threshold = admin.topicPolicies() + .getCompactionThreshold(ServiceUnitStateChannelImpl.TOPIC, false).longValue(); + assertEquals(5 * 1024 * 1024, threshold); + } catch (Exception e) { + ; + } + + }); + } + + @Test(priority = 2) + public void assignmentTest() + throws ExecutionException, InterruptedException, IllegalAccessException, TimeoutException { + var getOwnerRequests1 = spy(getOwnerRequests((channel1))); + var getOwnerRequests2 = spy(getOwnerRequests((channel2))); + + var owner1 = channel1.getOwnerAsync(bundle); + var owner2 = channel2.getOwnerAsync(bundle); + + assertNull(owner1); + assertNull(owner2); + + var assigned1 = channel1.publishAssignEventAsync(bundle, lookupServiceAddress1); + var assigned2 = channel2.publishAssignEventAsync(bundle, lookupServiceAddress2); + assertNotNull(assigned1); + assertNotNull(assigned2); + waitUntilOwnerChanges(channel2, bundle, null); Review Comment: I think adding the `Command` layer is additional complexity and requires more lines of code. Also, the current approach does not need to store the additional `Command` info in the system topic(space optimized). Regarding `ServiceUnitState.isValidTransition()`, the state validation(aka compaction) will happen in the tableview and topic compaction by `TopicCompactionStrategy`(which is in a separate PIP and PR here, https://github.com/apache/pulsar/issues/18099). This validation logic, ServiceUnitChannelCompactionStrategy will be added after this PR https://github.com/apache/pulsar/pull/18195 gets merged. -- 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]
