Copilot commented on code in PR #4657:
URL: https://github.com/apache/solr/pull/4657#discussion_r3640440285
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -387,7 +370,7 @@ private void distribute(FileInfo info) {
} else {
if (i == FETCHFROM_SRC) {
// This is just an optimization
- // at this point a bunch of nodes are already downloading from me
+ // at this point a bunch of nodes are already downloading from me.
Review Comment:
The comment "it's there in the memory now" is now misleading because the
in-memory `tmpFiles` cache was removed in this change. Please update the
comment so it matches current behavior (local on-disk availability).
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -112,13 +110,6 @@ class FileInfo {
this.path = path;
}
Review Comment:
After removing `getFileData(...)`, the `fileData`/`metaData` fields are no
longer read anywhere (and the assignments in `persistToFile` only keep extra
references alive). Removing them will reduce confusion and avoid retaining
large buffers unnecessarily.
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -418,11 +401,7 @@ private void distribute(FileInfo info) {
.getUpdateExecutor()
.submit(
() -> {
- try {
- Thread.sleep(10 * 1000);
- } finally {
- tmpFiles.remove(info.path);
- }
+ Thread.sleep(10 * 1000);
return null;
});
Review Comment:
The cleanup task submitted in the `finally` block now only sleeps for 10
seconds and does nothing else (the former `tmpFiles` cleanup is gone). This
needlessly ties up an update-executor thread and can reduce throughput if many
files are distributed.
--
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]
