You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ddanielr (via GitHub)" <gi...@apache.org> on 2023/04/13 23:05:47 UTC

[GitHub] [accumulo] ddanielr opened a new pull request, #3296: Fix MetricIT test metric line length

ddanielr opened a new pull request, #3296:
URL: https://github.com/apache/accumulo/pull/3296

   This issue was found during work on #3288 where metric lines with tags were showing a length of over `106000`.
   
   This lead to the discovery that all metric lines were being stored with a string length of `106496`. 
   
   This value comes from a Datagram Socket's expected receive buffer size.  https://github.com/apache/accumulo/blob/92238929f63e8ae075120712317e4cb5636f029f/test/src/main/java/org/apache/accumulo/test/metrics/TestStatsDSink.java#L98
   
   Since `packet.getLength()` is updated on the `sock.receive(packet)` call, the code now uses that length to trim the buffer down to the expected data size. 
   
   Removes the trim call from `parseStatsDMetric()` as metric lines are now the correct size.


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] EdColeman commented on pull request #3296: Fix MetricIT test metric line length

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on PR #3296:
URL: https://github.com/apache/accumulo/pull/3296#issuecomment-1509165407

   I include the fix in #3297 


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3296: Fix MetricIT test metric line length

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3296:
URL: https://github.com/apache/accumulo/pull/3296#discussion_r1166123380


##########
test/src/main/java/org/apache/accumulo/test/metrics/TestStatsDSink.java:
##########
@@ -102,7 +103,8 @@ public TestStatsDSink() throws SocketException {
         DatagramPacket packet = new DatagramPacket(buf, len);
         try {
           sock.receive(packet);
-          received.add(new String(packet.getData()));
+          byte[] trimmedBuffer = Arrays.copyOf(packet.getData(), packet.getLength());
+          received.add(new String(trimmedBuffer));

Review Comment:
   Could this use the String constructor that takes start and length, something like 
   ```
             received.add(new String(packet.getData(),0, packet.getLength()));
   ```
   That would avoid an extra copy operation



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] EdColeman closed pull request #3296: Fix MetricIT test metric line length

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman closed pull request #3296: Fix MetricIT test metric line length
URL: https://github.com/apache/accumulo/pull/3296


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org