-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
List all PITs service layer changes #4016
List all PITs service layer changes #4016
Conversation
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
…ch into deletepitservice
Gradle Check (Jenkins) Run Completed with:
|
f9811be
to
652b4ab
Compare
Gradle Check (Jenkins) Run Completed with:
|
652b4ab
to
a0216d1
Compare
Gradle Check (Jenkins) Run Completed with:
|
a0216d1
to
0282458
Compare
Gradle Check (Jenkins) Run Completed with:
|
0282458
to
7f78b0b
Compare
Gradle Check (Jenkins) Run Completed with:
|
7f78b0b
to
d69e5d9
Compare
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
d69e5d9
to
17fffdc
Compare
Gradle Check (Jenkins) Run Completed with:
|
public class GetAllPitNodeRequest extends BaseNodeRequest { | ||
GetAllPitNodesRequest request; | ||
|
||
@Inject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Inject
seems to be not needed here, right?
* Inner node get all pits request | ||
*/ | ||
public class GetAllPitNodeRequest extends BaseNodeRequest { | ||
GetAllPitNodesRequest request; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How request
is used? I have not found any references to it ...
*/ | ||
private List<ListPitInfo> pitsInfo; | ||
|
||
@Inject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, I believe @Inject
serves no purpose here, does it?
private List<ListPitInfo> pitsInfo; | ||
|
||
@Inject | ||
public GetAllPitNodeResponse(StreamInput in, List<ListPitInfo> pitsInfo) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, is this constructor even needed?
/** | ||
* List of unique PITs across all nodes | ||
*/ | ||
List<ListPitInfo> pitsInfo = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be private
?
} | ||
|
||
public List<ListPitInfo> getPITIDs() { | ||
return new ArrayList<>(pitsInfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collections.unmodifiableList(pitsInfo)
?
DiscoveryNode node = cursor.value; | ||
nodes.add(node); | ||
} | ||
DiscoveryNode[] disNodesArr = new DiscoveryNode[nodes.size()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, a bit more compact notation:
DiscoveryNode[] disNodesArr = nodes.toArray(new DiscoveryNode[nodes.size()]);
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Gradle Check (Jenkins) Run Completed with:
|
… into listallpitservice
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
*/ | ||
private final Set<ListPitInfo> pitInfos = new HashSet<>(); | ||
|
||
@Inject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we cleaned all unnecessary @Inject
, left overs?
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
1634963
to
7779b97
Compare
Gradle Check (Jenkins) Run Completed with:
|
… into listallpitservice
7779b97
to
ba620c7
Compare
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
… into listallpitservice
8e58f97
to
a6c073f
Compare
Gradle Check (Jenkins) Run Completed with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clearly document the current limitations with delete and List PIT for now.
Description
Changes to list all active point in time searches to the user. This contains only the service layer changes and rest layer changes will come in later.
Request:
localhost:9200/_search/point_in_time/all
Response:
'Include all' flag as part of request will also show node results and failures on top of the above response, which will be handled in rest layer.
Issues Resolved
#1147
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.