Skip to content

[client] release pending fetch semaphore on discarding#3043

Open
zuston wants to merge 1 commit intoapache:mainfrom
zuston:fixloghang
Open

[client] release pending fetch semaphore on discarding#3043
zuston wants to merge 1 commit intoapache:mainfrom
zuston:fixloghang

Conversation

@zuston
Copy link
Copy Markdown
Member

@zuston zuston commented Apr 9, 2026

Purpose

Linked issue: close #xxx

Brief change log

Tests

API and Format

Documentation

Copy link
Copy Markdown
Contributor

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zuston Nice catch on the permit leak.

Could you add a regression test in LogFetchCollectorTest? CompletedFetch.drain() sets isConsumed=true, so the fix is observable with zero new helpers, I have smth like this in mind:

  @Test                                                                                                                                                                                                                                              
  void testCollectDrainsDiscardedFetch() throws Exception {    
      TableBucket tb = new TableBucket(DATA1_TABLE_ID, 0);                                                                                                                                                                                           
      CompletedFetch completedFetch =                                                                                                                                                                                                                
              makeCompletedFetch(
                      tb,                                                                                                                                                                                                                            
                      new FetchLogResultForBucket(tb,genMemoryLogRecordsByObject(DATA1), 10L), 0L);                                                                                                                                                                                                                           
      logFetchBuffer.add(completedFetch);
      logScannerStatus.unassignScanBuckets(Collections.singletonList(tb));                                                                                                                                                                           
                                                                                                                                                                                                                                                     
      ScanRecords records = logFetchCollector.collectFetch(logFetchBuffer);
                                                                                                                                                                                                                                                     
      assertThat(records.buckets()).isEmpty();                                                                                                                                                                                                       
      assertThat(completedFetch.isConsumed()).isTrue();
  }      

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants