github-advanced-security[bot] commented on code in PR #18176:
URL: https://github.com/apache/druid/pull/18176#discussion_r2206799588
##########
server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java:
##########
@@ -1047,190 +746,252 @@
SegmentLocalCacheManager.DOWNLOAD_START_MARKER_FILE_NAME
);
Assert.assertTrue(downloadMarker.createNewFile());
+ // create a new manager, expect corrupted segment to be cleaned out and
freshly downloaded after startup
+ SegmentLocalCacheManager manager = makeDefaultManager(jsonMapper);
Review Comment:
## Possible confusion of local and field
Confusing name: method [testGetSegmentFilesWhenDownloadStartMarkerExists](1)
also refers to field [manager](2) (without qualifying it with 'this').
[Show more
details](https://github.com/apache/druid/security/code-scanning/9416)
##########
server/src/test/java/org/apache/druid/segment/loading/SegmentLocalCacheManagerTest.java:
##########
@@ -1047,190 +746,252 @@
SegmentLocalCacheManager.DOWNLOAD_START_MARKER_FILE_NAME
);
Assert.assertTrue(downloadMarker.createNewFile());
+ // create a new manager, expect corrupted segment to be cleaned out and
freshly downloaded after startup
+ SegmentLocalCacheManager manager = makeDefaultManager(jsonMapper);
- Assert.assertFalse("Expect cache miss for corrupted segment file",
manager.isSegmentCached(segmentToDownload));
- Assert.assertFalse(cachedSegmentDir.exists());
- }
-
- @Test
- public void testReserveSegment()
- {
- final DataSegment dataSegment =
dataSegmentWithInterval("2014-10-20T00:00:00Z/P1D").withSize(100L);
- final StorageLocation firstLocation = new
StorageLocation(localSegmentCacheFolder, 200L, 0.0d);
- final StorageLocation secondLocation = new
StorageLocation(localSegmentCacheFolder, 150L, 0.0d);
-
- manager = new SegmentLocalCacheManager(
- Arrays.asList(secondLocation, firstLocation),
- new SegmentLoaderConfig(),
- new
RoundRobinStorageLocationSelectorStrategy(Arrays.asList(firstLocation,
secondLocation)),
- TestIndex.INDEX_IO,
- jsonMapper
- );
- Assert.assertTrue(manager.reserve(dataSegment));
-
Assert.assertTrue(firstLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(dataSegment,
false)));
- Assert.assertEquals(100L, firstLocation.availableSizeBytes());
- Assert.assertEquals(150L, secondLocation.availableSizeBytes());
-
- // Reserving again should be no-op
- Assert.assertTrue(manager.reserve(dataSegment));
-
Assert.assertTrue(firstLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(dataSegment,
false)));
- Assert.assertEquals(100L, firstLocation.availableSizeBytes());
- Assert.assertEquals(150L, secondLocation.availableSizeBytes());
-
- // Reserving a second segment should now go to a different location
- final DataSegment otherSegment =
dataSegmentWithInterval("2014-10-21T00:00:00Z/P1D").withSize(100L);
- Assert.assertTrue(manager.reserve(otherSegment));
-
Assert.assertTrue(firstLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(dataSegment,
false)));
-
Assert.assertFalse(firstLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(otherSegment,
false)));
-
Assert.assertTrue(secondLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(otherSegment,
false)));
- Assert.assertEquals(100L, firstLocation.availableSizeBytes());
- Assert.assertEquals(50L, secondLocation.availableSizeBytes());
+ manager.load(segmentToDownload);
+ manager.getSegmentFiles(segmentToDownload);
+ // this is still true becuase
+ Assert.assertTrue("Don't expect cache miss for corrupted segment file",
manager.isSegmentCached(segmentToDownload));
+ Assert.assertTrue(cachedSegmentDir.exists());
+ Assert.assertFalse(downloadMarker.exists());
}
@Test
- public void testReserveNotEnoughSpace()
+ public void testGetBootstrapSegment() throws SegmentLoadingException
{
- final DataSegment dataSegment =
dataSegmentWithInterval("2014-10-20T00:00:00Z/P1D").withSize(100L);
- final StorageLocation firstLocation = new
StorageLocation(localSegmentCacheFolder, 50L, 0.0d);
- final StorageLocation secondLocation = new
StorageLocation(localSegmentCacheFolder, 150L, 0.0d);
+ final ObjectMapper jsonMapper = TestHelper.makeJsonMapper();
+ jsonMapper.registerSubtypes(TestSegmentUtils.TestLoadSpec.class);
+ jsonMapper.registerSubtypes(TestSegmentUtils.TestSegmentizerFactory.class);
- manager = new SegmentLocalCacheManager(
- Arrays.asList(secondLocation, firstLocation),
- new SegmentLoaderConfig(),
- new
RoundRobinStorageLocationSelectorStrategy(Arrays.asList(firstLocation,
secondLocation)),
- TestIndex.INDEX_IO,
+ final StorageLocationConfig locationConfig = new
StorageLocationConfig(localSegmentCacheFolder, 10000L, null);
+ final SegmentLoaderConfig loaderConfig = new
SegmentLoaderConfig().withLocations(ImmutableList.of(locationConfig));
+ final List<StorageLocation> storageLocations =
loaderConfig.toStorageLocations();
+ SegmentLocalCacheManager manager = new SegmentLocalCacheManager(
+ storageLocations,
+ loaderConfig,
+ new LeastBytesUsedStorageLocationSelectorStrategy(storageLocations),
+ TestHelper.getTestIndexIO(jsonMapper, ColumnConfig.DEFAULT),
jsonMapper
);
- // should go to second location if first one doesn't have enough space
- Assert.assertTrue(manager.reserve(dataSegment));
-
Assert.assertTrue(secondLocation.isReserved(DataSegmentPusher.getDefaultStorageDir(dataSegment,
false)));
- Assert.assertEquals(50L, firstLocation.availableSizeBytes());
- Assert.assertEquals(50L, secondLocation.availableSizeBytes());
+ final DataSegment dataSegment = TestSegmentUtils.makeSegment("foo", "v1",
Intervals.of("2020/2021"));
- final DataSegment otherSegment =
dataSegmentWithInterval("2014-10-21T00:00:00Z/P1D").withSize(100L);
- Assert.assertFalse(manager.reserve(otherSegment));
- Assert.assertEquals(50L, firstLocation.availableSizeBytes());
- Assert.assertEquals(50L, secondLocation.availableSizeBytes());
+ manager.bootstrap(dataSegment, SegmentLazyLoadFailCallback.NOOP);
+ Segment actualBootstrapSegment =
manager.acquireSegment(dataSegment).orElse(null);
+ Assert.assertNotNull(actualBootstrapSegment);
+ Assert.assertEquals(dataSegment.getId(), actualBootstrapSegment.getId());
+ Assert.assertEquals(dataSegment.getInterval(),
actualBootstrapSegment.getDataInterval());
}
@Test
- public void testSegmentDownloadWhenLocationReserved() throws Exception
+ public void testGetSegmentVirtualStorageFabric() throws Exception
{
- final List<StorageLocationConfig> locationConfigs = new ArrayList<>();
- final StorageLocationConfig locationConfig =
createStorageLocationConfig("local_storage_folder", 10000000000L, true);
- final StorageLocationConfig locationConfig2 =
createStorageLocationConfig("local_storage_folder2", 1000000000L, true);
- final StorageLocationConfig locationConfig3 =
createStorageLocationConfig("local_storage_folder3", 1000000000L, true);
- locationConfigs.add(locationConfig);
- locationConfigs.add(locationConfig2);
- locationConfigs.add(locationConfig3);
-
- List<StorageLocation> locations = new ArrayList<>();
- for (StorageLocationConfig locConfig : locationConfigs) {
- locations.add(
- new StorageLocation(
- locConfig.getPath(),
- locConfig.getMaxSize(),
- locConfig.getFreeSpacePercent()
- )
- );
- }
+ final StorageLocationConfig locationConfig = new
StorageLocationConfig(localSegmentCacheFolder, 10000L, null);
+ final SegmentLoaderConfig loaderConfig = new SegmentLoaderConfig()
+ {
+ @Override
+ public List<StorageLocationConfig> getLocations()
+ {
+ return ImmutableList.of(locationConfig);
+ }
Review Comment:
## Ignored error status of call
Method createStorageLocationConfig ignores exceptional return value of
File.setWritable.
[Show more
details](https://github.com/apache/druid/security/code-scanning/2610)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
