This is an automated email from the ASF dual-hosted git repository. yong pushed a commit to branch branch-4.15 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 494f9e949930614031f5b4197694961a39c3d57b Author: wenbingshen <[email protected]> AuthorDate: Wed Nov 9 09:09:26 2022 +0800 Fix GetBookieInfo failed event stats (#3622) ### Motivation In `GetBookieInfoProcessorV3`, even if throw `IOException` and get DiskSpace failed, it will still `registerSuccessfulEvent`, which is wrong, we should `registerFailedEvent`. ### Changes 1. registerFailedEvent when get DiskSpace failed. 2. add unit test. (cherry picked from commit 74e9ef4d732622fba6cf023cc26ce9a07e066af0) --- .../bookkeeper/proto/GetBookieInfoProcessorV3.java | 6 +- .../proto/GetBookieInfoProcessorV3Test.java | 101 +++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3.java index fe315f62ca..9978e6c448 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3.java @@ -74,14 +74,16 @@ public class GetBookieInfoProcessorV3 extends PacketProcessorBaseV3 implements R getBookieInfoResponse.setTotalDiskCapacity(totalDiskSpace); } LOG.debug("FreeDiskSpace info is " + freeDiskSpace + " totalDiskSpace is: " + totalDiskSpace); + requestProcessor.getRequestStats().getGetBookieInfoStats() + .registerSuccessfulEvent(MathUtils.elapsedNanos(startTimeNanos), TimeUnit.NANOSECONDS); } catch (IOException e) { status = StatusCode.EIO; LOG.error("IOException while getting freespace/totalspace", e); + requestProcessor.getRequestStats().getGetBookieInfoStats() + .registerFailedEvent(MathUtils.elapsedNanos(startTimeNanos), TimeUnit.NANOSECONDS); } getBookieInfoResponse.setStatus(status); - requestProcessor.getRequestStats().getGetBookieInfoStats() - .registerSuccessfulEvent(MathUtils.elapsedNanos(startTimeNanos), TimeUnit.NANOSECONDS); return getBookieInfoResponse.build(); } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3Test.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3Test.java new file mode 100644 index 0000000000..e294a55ccf --- /dev/null +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/GetBookieInfoProcessorV3Test.java @@ -0,0 +1,101 @@ +/* + * 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.bookkeeper.proto; + +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.netty.channel.Channel; +import java.io.IOException; +import java.util.concurrent.TimeUnit; +import org.apache.bookkeeper.bookie.Bookie; +import org.apache.bookkeeper.stats.OpStatsLogger; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit test {@link GetBookieInfoProcessorV3}. + */ +public class GetBookieInfoProcessorV3Test { + + private Channel channel; + private BookieRequestProcessor requestProcessor; + private Bookie bookie; + private RequestStats requestStats; + private OpStatsLogger getBookieInfoStats; + private OpStatsLogger channelWriteStats; + private OpStatsLogger getBookieInfoRequestStats; + + @Before + public void setup() { + getBookieInfoStats = mock(OpStatsLogger.class); + channelWriteStats = mock(OpStatsLogger.class); + getBookieInfoRequestStats = mock(OpStatsLogger.class); + requestStats = mock(RequestStats.class); + requestProcessor = mock(BookieRequestProcessor.class); + bookie = mock(Bookie.class); + when(requestProcessor.getBookie()).thenReturn(bookie); + channel = mock(Channel.class); + when(channel.isOpen()).thenReturn(true); + when(channel.isActive()).thenReturn(true); + when(requestProcessor.getRequestStats()).thenReturn(requestStats); + when(requestProcessor.getRequestStats().getGetBookieInfoStats()) + .thenReturn(getBookieInfoStats); + when(requestProcessor.getRequestStats().getChannelWriteStats()) + .thenReturn(channelWriteStats); + when(requestProcessor.getRequestStats().getGetBookieInfoRequestStats()) + .thenReturn(getBookieInfoRequestStats); + } + + @Test + public void testGetBookieInfoProcessorStats() throws IOException { + final BookkeeperProtocol.BKPacketHeader.Builder headerBuilder = + BookkeeperProtocol.BKPacketHeader.newBuilder() + .setVersion(BookkeeperProtocol.ProtocolVersion.VERSION_THREE) + .setOperation(BookkeeperProtocol.OperationType.GET_BOOKIE_INFO) + .setTxnId(0); + + final BookkeeperProtocol.GetBookieInfoRequest.Builder getBookieInfoBuilder = + BookkeeperProtocol.GetBookieInfoRequest.newBuilder() + .setRequested(BookkeeperProtocol.GetBookieInfoRequest.Flags.FREE_DISK_SPACE_VALUE); + + final BookkeeperProtocol.Request getBookieInfoRequest = BookkeeperProtocol.Request.newBuilder() + .setHeader(headerBuilder) + .setGetBookieInfoRequest(getBookieInfoBuilder) + .build(); + + GetBookieInfoProcessorV3 getBookieInfo = new GetBookieInfoProcessorV3( + getBookieInfoRequest, channel, requestProcessor); + getBookieInfo.run(); + + // get BookieInfo succeeded. + verify(getBookieInfoStats, times(1)) + .registerSuccessfulEvent(anyLong(), eq(TimeUnit.NANOSECONDS)); + + // get BookieInfo failed. + when(requestProcessor.getBookie().getTotalFreeSpace()).thenThrow(new IOException("test for failed.")); + getBookieInfo.run(); + verify(getBookieInfoStats, times(1)) + .registerFailedEvent(anyLong(), eq(TimeUnit.NANOSECONDS)); + } +} \ No newline at end of file
