psalagnac commented on code in PR #4657:
URL: https://github.com/apache/solr/pull/4657#discussion_r3675874185
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -367,65 +352,48 @@ private void distribute(FileInfo info) {
} catch (Exception e) {
throw new SolrException(SERVER_ERROR, "Unable to create an entry in ZK",
e);
}
- tmpFiles.put(info.path, info);
List<String> nodes =
FileStoreUtils.fetchAndShuffleRemoteLiveNodes(coreContainer);
int i = 0;
int FETCHFROM_SRC = 50;
String myNodeName = coreContainer.getZkController().getNodeName();
- String getFrom = "";
- try {
- for (String node : nodes) {
- String baseUrl =
-
coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node);
-
- String nodeToFetchFrom;
- if (i < FETCHFROM_SRC) {
- // this is to protect very large clusters from overwhelming a single
node
- // the first FETCHFROM_SRC nodes will be asked to fetch from this
node.
- // it's there in the memory now. So , it must be served fast
- nodeToFetchFrom = myNodeName;
- } else {
- if (i == FETCHFROM_SRC) {
- // This is just an optimization
- // at this point a bunch of nodes are already downloading from me
- // I'll wait for them to finish before asking other nodes to
download from each other
- try {
- Thread.sleep(2 * 1000);
- } catch (Exception e) {
- }
+ for (String node : nodes) {
+ String baseUrl =
+
coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node);
+
+ String nodeToFetchFrom;
+ if (i < FETCHFROM_SRC) {
+ // this is to protect very large clusters from overwhelming a single
node
+ // the first FETCHFROM_SRC nodes will be asked to fetch from this node.
+ // it's there in the memory now. So, it must be served fast
+ nodeToFetchFrom = myNodeName;
+ } else {
+ if (i == FETCHFROM_SRC) {
+ // This is just an optimization
+ // at this point a bunch of nodes are already downloading from me.
+ // I'll wait for them to finish before asking other nodes to
download from each other
+ try {
+ Thread.sleep(2 * 1000);
+ } catch (Exception e) {
Review Comment:
Not new with this change, but I noticed this try/catch is unsafe.
It is required to at least set the interrupted flag.
```suggestion
Thread.sleep(2 * 1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
```
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -112,13 +110,6 @@ class FileInfo {
this.path = path;
}
Review Comment:
That's correct. The two buffers are now unused and can be removed.
##########
solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java:
##########
@@ -367,65 +352,48 @@ private void distribute(FileInfo info) {
} catch (Exception e) {
throw new SolrException(SERVER_ERROR, "Unable to create an entry in ZK",
e);
}
- tmpFiles.put(info.path, info);
List<String> nodes =
FileStoreUtils.fetchAndShuffleRemoteLiveNodes(coreContainer);
int i = 0;
int FETCHFROM_SRC = 50;
String myNodeName = coreContainer.getZkController().getNodeName();
- String getFrom = "";
- try {
Review Comment:
This looks good to me.
Now that `tmpFiles` is removed, this async task does nothing else than
sleeping.
I'm good with this removal.
--
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]
