Skip to content
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

Enhancement: Introduce Simultaneous Querying for Alarms with Deviation >= and <= in IHR API #48

Open
mohamedawnallah opened this issue Jan 14, 2024 · 3 comments · May be fixed by #59
Open

Comments

@mohamedawnallah
Copy link
Member

Problem

Currently, our system faces a slowdown issue due to the need for two network requests to fetch hegemony alarms with a deviation >= 20 and <= -20. Given the abundance of hegemony alarms, this process becomes inefficient. The existing IHR API cannot simultaneously query both deviation values, such as >= 20 and <= -20.

Feature Description

To address the inefficiency, a new feature needs to be implemented in the IHR API. This feature should enable the querying of both deviation values concurrently, for example, >= value1 and <= -value2. This enhancement aims to significantly improve the speed of retrieving hegemony alarms.

Additional Context

To better understand the current implementation, refer to the code snippet in ihr-website responsible for making two separate network requests to obtain hegemony alarms with deviation >= 20 and <= -20:
https://github.com/InternetHealthReport/ihr-website/blob/cae2f402dca4cb7efb74508c4bedc059a1186ae0/src/views/GlobalReport.vue#L182-L190

@mohamedawnallah mohamedawnallah changed the title Enhancement: Introduce Simultaneous Querying for Hegemony Alarms with Deviation >= and <= in IHR API Enhancement: Introduce Simultaneous Querying for Alarms with Deviation >= and <= in IHR API Jan 14, 2024
@MAVRICK-1
Copy link

MAVRICK-1 commented Jan 28, 2024

To address the inefficiency in making two separate network requests for hegemony alarms with deviations >= 20 and <= -20, we can modify the code to use a single query for both deviation values concurrently. Here's the modified code:

  1. Update HegemonyAlarmsFilter in views.py:

    Add a deviation_combined field to HegemonyAlarmsFilter to handle combined deviation values.

    class HegemonyAlarmsFilter(HelpfulFilterSet):
        asn = ListIntegerFilter(help_text="ASN of the anomalous dependency (transit network). Can be a single value or a list of comma-separated values.")
        originasn = ListIntegerFilter(help_text="ASN of the reported dependent network. Can be a single value or a list of comma-separated values.")
        
        deviation_combined = ListFloatFilter(help_text="Combined deviation filter (e.g., [20, -20]).")
    
        class Meta:
            model = Hegemony_alarms
            fields = {
                'timebin': ['exact', 'lte', 'gte'],
                'af': ['exact'],
                'deviation': ['lte', 'gte'],
            }
            ordering_fields = ('timebin', 'originasn', 'deviation', 'af')
    
        filter_overrides = {
            django_models.DateTimeField: {
                'filter_class': filters.IsoDateTimeFilter
            },
        }
    
        def filter_deviation_combined(self, queryset, name, value):
            if value:
                queryset = queryset.filter(
                    Q(deviation__gte=value[0]) | Q(deviation__lte=-value[1])
                )
            return queryset
    
    
  2. Update HegemonyAlarmsView in views.py:

    Modify the get_queryset method to use the new filter_deviation_combined method.

    class HegemonyAlarmsView(generics.ListAPIView):
        """
        List significant AS dependency changes detected by IHR anomaly detector.
        <ul>
        <li><b>Required parameters:</b> timebin or a range of timebins (using the two parameters timebin__lte and timebin__gte).</li>
        <li><b>Limitations:</b> At most 7 days of data can be fetched per request. For bulk downloads see: <a href="https://ihr-archive.iijlab.net/" target="_blank">https://ihr-archive.iijlab.net/</a>.</li>
        </ul>
        """
        serializer_class = HegemonyAlarmsSerializer
        filter_class = HegemonyAlarmsFilter
    
        def list(self, request, *args, **kwargs):
            response = super().list(request, *args, **kwargs)
            last = self.request.query_params.get('timebin', 
                    self.request.query_params.get('timebin__gte', None) )
            if last is not None:
                # Cache forever content that is more than a week old
                today = date.today()
                past_days = today - timedelta(days=7) 
                if arrow.get(last).date() < past_days: 
                    patch_cache_control(response, max_age=15552000)
    
            return response
    
        def get_queryset(self):
            check_timebin(self.request.query_params)
            queryset = Hegemony_alarms.objects.all()
    
            # Use filter_deviation_combined from HegemonyAlarmsFilter
            filter_instance = HegemonyAlarmsFilter(self.request.query_params, queryset=queryset)
            queryset = filter_instance.filter_deviation_combined(queryset, 'deviation_combined', self.request.query_params.getlist('deviation_combined'))
    
            return queryset```

With these changes, you can now query both deviation values concurrently, improving the efficiency of retrieving hegemony alarms.
Is this approach okay @mohamedawnallah ?
can you assign me this :-)?

@richardshaju
Copy link

I think to run both hegemonyAlarmsFilter1 and hegemonyAlarmsFilter2 simultaneously, we can use Promise.all() to run them concurrently.

const hegemonyAlarmsFilters = computed(async () => {
  const hegemonyAlarmsFilter1 = new HegemonyAlarmsQuery()
    .deviation(20, Query.GTE)
    .timeInterval(startTime.value, endTime.value);
  
  const hegemonyAlarmsFilter2 = new HegemonyAlarmsQuery()
    .deviation(-20, Query.LTE)
    .timeInterval(startTime.value, endTime.value);
  
  const [result1, result2] = await Promise.all([
    hegemonyAlarmsFilter1.run(),
    hegemonyAlarmsFilter2.run()
  ]);

  return [result1, result2];
});

@Forchapeatl
Copy link
Contributor

Forchapeatl commented Feb 26, 2024

Hello everyone, may I work on this issue ?

@Forchapeatl Forchapeatl linked a pull request Mar 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants